calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Altekruse <altekruseja...@gmail.com>
Subject Re: Support to reset an option
Date Wed, 29 Jul 2015 22:17:13 GMT
I do understand that the most correct version of this fix is just to return
a different identifier type in these two cases, the same as it is with
column names. I was just suggesting possibly standardizing on the parse
tree from these two cases actually returning identical results, as Drill is
going to need to make these two inter-operate for backward compatibility
anyway.

On the other hand I don't generally see it as advisable to lose
information, so it is probably best for these to be handled separately as
far as calcite is concerned and any cross-compatibility to be managed by
Drill and other consumers.

On Wed, Jul 29, 2015 at 2:47 PM, Julian Hyde <jhyde@apache.org> wrote:

> You get a different parse tree depending on whether the dots are inside or
> outside the back-ticks.
>
>   ALTER SYSTEM SET `foo`.`bar`.`baz` = 1
>
> gives a 3-part identifier with parts “foo”, “bar”, “baz", while
>
>   ALTER SYSTEM SET `foo.bar.baz` = 1
>
> gives a 1-part identifier with name “foo.bar.baz”. I don’t know whether
> you want the first variant, but it is not supported right now.
>
> Julian
>
>
> > On Jul 28, 2015, at 1:40 PM, Jason Altekruse <altekrusejason@gmail.com>
> wrote:
> >
> > To support the second case I was mentioning it would actually require a
> > change in calcite that I was not thinking about, in terms of adding a new
> > compound identifier to the sql parse tree node you show here. I don't
> think
> > there is a need for this functionality, I guess the take away is I think
> > that we just need to make sure we use standardized format of the
> flattened
> > compound names into this name field when creating the sql parse tree. I
> > know the parser already hides details of removing backtick, so in all
> > likelihood it will just work as expected. It would probably just be worth
> > writing unit tests that use both strategies and make sure the names
> > provided from the parse step are the same.
> >
> > On Tue, Jul 28, 2015 at 12:28 PM, Julian Hyde <jhyde@apache.org> wrote:
> >
> >> Jason,
> >>
> >> I think Drill is the only database using this feature so I don’t think
> >> compatibility is too important.
> >>
> >> What change would be needed? We have
> >>
> >> public class SqlSetOption extends SqlCall {
> >>  /** Scope of the assignment. Values "SYSTEM" and "SESSION" are typical.
> >> */
> >>  String scope;
> >>
> >>  String name;
> >>
> >>  /** Value of the option. May be a {@link
> >> org.apache.calcite.sql.SqlLiteral} or
> >>   * a {@link org.apache.calcite.sql.SqlIdentifier} with one
> >>   * part. Reserved words (currently just 'ON') are converted to
> >>   * identifiers by the parser. */
> >>  SqlNode value;
> >> }
> >>
> >> and we could add a “SqlIdentifier identifier” field and deprecate
> “name”.
> >> “name” could continue to hold the last part of the identifier in the
> short
> >> term.
> >>
> >> If a particular database built using Calcite only allows single-part
> >> identifiers for options then it can easily throw a validation error.
> >>
> >> Julian
> >>
> >>
> >>
> >>> On Jul 28, 2015, at 12:11 PM, Jason Altekruse <
> altekrusejason@gmail.com>
> >> wrote:
> >>>
> >>> One note on Jacques point about the compound identifiers. To maintain
> >>> backwards compatibility we are going to need to fudge these new
> multipart
> >>> identifiers together with the old single part identifiers that were
> using
> >>> previously.
> >>>
> >>> In particular for Drill, this could be as simple as maintaining the
> >> current
> >>> data structure, which is just a map between strings for option names
> and
> >>> option values themselves, with the extra consideration that compound
> >> names
> >>> will need to be canonicalized into a standard singular string. The
> >>> alternative would be to replace the structure with a map from a nested
> >> name
> >>> representation to option values, with all of the flattened option names
> >>> registered at the root. This is something that can be handled by each
> >> user
> >>> of calcite individually, but I thought it might be worth thinking about
> >> the
> >>> best way to advise these two cases working together now that there is a
> >>> need for backwards compatibility.
> >>>
> >>> On Tue, Jul 28, 2015 at 8:15 AM, Julian Hyde <jhyde@apache.org> wrote:
> >>>
> >>>> Create a test case similar to SqlParserTest.testSqlOptions... the rest
> >>>> should follow...
> >>>>
> >>>> On Tue, Jul 28, 2015 at 6:07 AM, Jacques Nadeau <jacques@apache.org>
> >>>> wrote:
> >>>>> Ps, the word at the beginning of that email should have been "if"
> >>>>> On Jul 28, 2015 6:06 AM, "Jacques Nadeau" <jacques@apache.org>
> wrote:
> >>>>>
> >>>>>> Of you're making changes there anyway,  can you make two additional
> >>>>>> changes:
> >>>>>>
> >>>>>> Allow "alter session" to be optional for setting.
> >>>>>> Allow a multipart identifier (so we don't have to quote a.b.c
(same
> as
> >>>>>> Schema or column identifiers in project lists).
> >>>>>>
> >>>>>> This would substantially improve usability.
> >>>>>> On Jul 27, 2015 6:53 PM, "Sudheesh Katkam" <skatkam@maprtech.com>
> >>>> wrote:
> >>>>>>
> >>>>>>> Hello developers,
> >>>>>>>
> >>>>>>> To support these statements as part of Apache Drill's SQL
parser
> >>>>>>> extension:
> >>>>>>>
> >>>>>>> ALTER SESSION RESET `option name`
> >>>>>>> ALTER SYSTEM RESET `option name`
> >>>>>>> ALTER SESSION RESET ALL
> >>>>>>> ALTER SYSTEM RESET ALL
> >>>>>>>
> >>>>>>> I added the required implementation files, including changes
to the
> >>>>>>> config file (keywords, statementParserMethods fields). However,
I
> >> get a
> >>>>>>> ParseException because the parser resolves to SetSqlOption
> statement,
> >>>> which
> >>>>>>> I assume is due to a lookahead of 2. How do I go about resolving
> >> this?
> >>>>>>>
> >>>>>>> Thank you,
> >>>>>>> Sudheesh
> >>>>>>
> >>>>>>
> >>>>
> >>
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message