From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
Cc: | Peter Moser <pitiz29a(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] [PROPOSAL] Temporal query processing with range types |
Date: | 2018-01-08 16:18:47 |
Message-ID: | CA+TgmoZbKoGvuGvJHBpY9s8AbLkPv8SemsSwtwg2o-id8Z3a7Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jan 6, 2018 at 3:29 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> I'm not very keen on adopting new syntax that isn't in the
> SQLStandard. They have a bad habit of doing something completely
> different. So a flexible approach will allow us to have functionality
> now and we can adopt any new syntax later.
>
> For any new syntax, I think the right approach would be to create a
> new parser plugin. That way we could make all of this part of an
> extension.
> * a parser plugin for any new syntax
> * various PostJoinSetProjection() functions to be called as needed
> So the idea is we enable Postgres to allow major new functionality, as
> was done for PostGIS so successfully.
>
> We can adopt syntax into the main parser later once SQLStandard
> accepts this, or some munged version of it.
We don't currently have any concept of a parser plugin, and I don't
think it's reasonable for this patch to try to invent one. I can't
see where you could possibly put a hook that would handle something
like this. I've thought about suggesting a hook that gets called if
parsing fails, before we give up and throw a syntax error. That
would, perhaps, allow extension to implement additional DDL commands,
which would be pretty cool. However, it wouldn't be practical to use
something like that for this because this is syntax that appears
buried deep down inside FROM clauses, and you'd basically have to
reimplement the core parser's entire handling of SELECT statements,
which would be immensely complicated to develop and a real pain to
maintain.
Furthermore, the changes here aren't only parsing changes. There is a
proposal to add a new executor node which has to be supported by new
planner code. A great deal more than parser pluggability would be
needed. And if we did all that, it doesn't really solve anything
anyway: the potential problem with committing to this syntax is that
if we change it later, there could be upgrade problems.
Extension-izing this wouldn't make those problems go away. People are
going to want to be able to run pg_dump on their old database and
restore the result on their new database, full stop. If we add this
syntax, in core or in a hypothetical extension system, we're stuck
with it and must maintain it going forward.
I also don't agree with the idea that we should reject syntax that
doesn't appear in the SQL standard. We have a great deal of such
syntax already, and we add more of it in every release, and a good
deal of it is contributed by you and your colleagues. I don't see why
this patch should be held to a stricter standard than we do in
general. I agree that there is some possibility for pain if the SQL
standards committee adopts syntax that is similar to whatever we pick
but different in detail, but I don't think we should be too worried
about that unless other database systems, such as Oracle, have syntax
that is similar to what is proposed here but different in detail. The
SQL standards committee seems to like standardizing on whatever
companies with a lot of money have already implemented; it's unlikely
that they are going to adopt something totally different from any
existing system but inconveniently similar to ours.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2018-01-08 16:27:40 | Re: [HACKERS] Refactoring identifier checks to consistently use strcmp |
Previous Message | Tom Lane | 2018-01-08 16:01:53 | Buildfarm status monitoring (was Re: pgsql: Implement channel binding tls-server-end-point for SCRAM) |