You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Shengkai Fang <fs...@gmail.com> on 2021/04/20 13:39:26 UTC

Override the CalciteResources.properties

Hi, devs.

Recently I am working around to improve the exception messages when using
parser to parse meaningless statement. For example,

```
parser.parse("INVALID COMMAND");
```
we will get the cause

```
org.apache.flink.table.api.SqlParserException: SQL parse failed. Non-query
expression encountered in illegal context
```

I think `Unsupprorted input statement` is much more straighforward for usrs
to understand.

So my goal is to find  how we can define the user-defined exception
message, i.e. define exception message for
`CalciteResource#illegalNonQueryExpression`.

After the dig, I find the `org.apache.calcite.runtime.Resources#create`
will try to load the content of  `CalciteResource.properties` to define the
exception message. If the `CalciteResource.properties` is not found or not
found the key in the file, it will use the anotation as the exception
message. The logic is located at
`org.apache.calcite.runtime.Resources$Inst#raw`

My solution to solve the problem is to override the
CalciteResources.properties and add the expected exception message in the
new CalciteResources.properties.

However, the new CalciteResources.properties doesn't `extend` the old
CalciteResources.properties. That is we lose other error messages that are
defined in the origin CalciteResources.properties. Do we need to copy the
origin content into the new file and modify the message we want to modify?

Look forward to your feedback.

Best,
Shengkai

Re: Override the CalciteResources.properties

Posted by Shengkai Fang <fs...@gmail.com>.
Hi, Julian.

Thanks for your explanation and suggestions. I will take it into
consideration.

Best,
Shengkai

Julian Hyde <jh...@apache.org> 于2021年4月21日周三 上午12:19写道:

> If there are some major improvements, consider proposing them as a
> Calcite change. You'd have to figure out whether your new message is
> an improvement in EVERY case where we currently throw "Non-query
> expression encountered in illegal context". That error happens in a
> tricky parser context, where we can't figure out whether we're looking
> at a query or an expression. For example, something like
>
>   SELECT * FROM t
>   WHERE x IN (((FOO BAR)))
>
> Note that where FOO occurs you could also have SELECT (because a
> sub-query could happen here) and you could also have an expression
> such as "1 + 2".
>
> Maybe the solution is to change the parser. If you know you are
> parsing a top-level statement (DDL command or query) and an expression
> is not possible you can give a better error.
>
> Julian
>
> On Tue, Apr 20, 2021 at 7:51 AM Shengkai Fang <fs...@gmail.com> wrote:
> >
> > Hi, devs.
> >
> > Recently I am working around to improve the exception messages when using
> > parser to parse meaningless statement. For example,
> >
> > ```
> > parser.parse("INVALID COMMAND");
> > ```
> > we will get the cause
> >
> > ```
> > org.apache.flink.table.api.SqlParserException: SQL parse failed.
> Non-query
> > expression encountered in illegal context
> > ```
> >
> > I think `Unsupprorted input statement` is much more straighforward for
> usrs
> > to understand.
> >
> > So my goal is to find  how we can define the user-defined exception
> > message, i.e. define exception message for
> > `CalciteResource#illegalNonQueryExpression`.
> >
> > After the dig, I find the `org.apache.calcite.runtime.Resources#create`
> > will try to load the content of  `CalciteResource.properties` to define
> the
> > exception message. If the `CalciteResource.properties` is not found or
> not
> > found the key in the file, it will use the anotation as the exception
> > message. The logic is located at
> > `org.apache.calcite.runtime.Resources$Inst#raw`
> >
> > My solution to solve the problem is to override the
> > CalciteResources.properties and add the expected exception message in the
> > new CalciteResources.properties.
> >
> > However, the new CalciteResources.properties doesn't `extend` the old
> > CalciteResources.properties. That is we lose other error messages that
> are
> > defined in the origin CalciteResources.properties. Do we need to copy the
> > origin content into the new file and modify the message we want to
> modify?
> >
> > Look forward to your feedback.
> >
> > Best,
> > Shengkai
>

Re: Override the CalciteResources.properties

Posted by Julian Hyde <jh...@apache.org>.
If there are some major improvements, consider proposing them as a
Calcite change. You'd have to figure out whether your new message is
an improvement in EVERY case where we currently throw "Non-query
expression encountered in illegal context". That error happens in a
tricky parser context, where we can't figure out whether we're looking
at a query or an expression. For example, something like

  SELECT * FROM t
  WHERE x IN (((FOO BAR)))

Note that where FOO occurs you could also have SELECT (because a
sub-query could happen here) and you could also have an expression
such as "1 + 2".

Maybe the solution is to change the parser. If you know you are
parsing a top-level statement (DDL command or query) and an expression
is not possible you can give a better error.

Julian

On Tue, Apr 20, 2021 at 7:51 AM Shengkai Fang <fs...@gmail.com> wrote:
>
> Hi, devs.
>
> Recently I am working around to improve the exception messages when using
> parser to parse meaningless statement. For example,
>
> ```
> parser.parse("INVALID COMMAND");
> ```
> we will get the cause
>
> ```
> org.apache.flink.table.api.SqlParserException: SQL parse failed. Non-query
> expression encountered in illegal context
> ```
>
> I think `Unsupprorted input statement` is much more straighforward for usrs
> to understand.
>
> So my goal is to find  how we can define the user-defined exception
> message, i.e. define exception message for
> `CalciteResource#illegalNonQueryExpression`.
>
> After the dig, I find the `org.apache.calcite.runtime.Resources#create`
> will try to load the content of  `CalciteResource.properties` to define the
> exception message. If the `CalciteResource.properties` is not found or not
> found the key in the file, it will use the anotation as the exception
> message. The logic is located at
> `org.apache.calcite.runtime.Resources$Inst#raw`
>
> My solution to solve the problem is to override the
> CalciteResources.properties and add the expected exception message in the
> new CalciteResources.properties.
>
> However, the new CalciteResources.properties doesn't `extend` the old
> CalciteResources.properties. That is we lose other error messages that are
> defined in the origin CalciteResources.properties. Do we need to copy the
> origin content into the new file and modify the message we want to modify?
>
> Look forward to your feedback.
>
> Best,
> Shengkai