You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Sergey Nuyanzin <sn...@gmail.com> on 2026/01/20 21:16:34 UTC
[QUESTION] Contract for SqlValidator#validate
Hi everyone
I faced behavior which seems weird to me, however I would like to
double check first.
Normally we call
SqlNode validatedNode = validator.validate(inputNode); //here
validator is SqlValidatorImpl
and everything is ok if we do not use inputNode (for whatever reason )
However if inputNode is used anywhere after validation then in some
cases validate method makes this node invalid by adding illegal nodes
From the javadoc for SqlValidator interface it is not clear if this
method is allowed to mutate SqlNode making it invalid or not[1]
Would be great to get better understanding whether it is expected
behavior (may be javadoc should be updated then) or not
P.S. Also I tweaked SqlValidatorTest in a way to make it run
validator.validate(inputNode) 2 times on the same input: there are a
few tests failed because of the things mentioned above
[1] https://github.com/apache/calcite/blob/270ba649cfa80228d3f4f6635a08ab06de504399/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java#L139-L146
--
Best regards,
Sergey
Re: [QUESTION] Contract for SqlValidator#validate
Posted by Julian Hyde <jh...@gmail.com>.
Mihai wrote:
> The validator does modify the expression tree, for
> example to insert explicit casts during type coercion.
> There is no way around this.
The idealist in me wishes that the validator were entirely read-only (populating a map of the type of each node); expansion of “*” and making implicit casts explicit could happen in later phases that create a copy of the AST.
I remember (pre-Calcite) versions of the validator that did surgery on the AST to do things like flatten and decorrelate subqueries. There were truly horrendous bugs, eliminated by making the validator mostly read-only.
Further investment would, I think, pay for itself.
Sergey wrote:
> if the input sqlNode is valid, after calling validate it
> should stay valid (even if changed)
Yes. I should have said so explicitly.
I don’t know your exact situation, but if it’s difficult to transform the tree while keeping it valid, maybe we need new validation rules (i.e. reject some queries that we currently consider valid).
Julian
> On Jan 21, 2026, at 1:43 AM, Sergey Nuyanzin <sn...@gmail.com> wrote:
>
> Thanks for your responses
>
> I will try to invest time here in my spare time and fix the cases (also
> will create jira issue for that)
>
> Best regards,
> Sergey
>
> On Wed, Jan 21, 2026, 10:27 Sergey Nuyanzin <sn...@gmail.com> wrote:
>
>> I agree that it might modify tree
>> my point is: if the input sqlNode is valid, after calling validate it
>> should stay valid (even if changed)
>> right now for some use cases it becomes invalid
>>
>> On Wed, Jan 21, 2026 at 10:03 AM Mihai Budiu <mb...@gmail.com> wrote:
>>>
>>> The validator does modify the expression tree, for example to insert
>> explicit casts during type coercion. There is no way around this. In
>> general, type inference may also modify the expression tree.
>>>
>>> Mihai
>>>
>>> ________________________________
>>> From: Ruben Q L <ru...@gmail.com>
>>> Sent: Wednesday, January 21, 2026 1:00 AM
>>> To: dev@calcite.apache.org <de...@calcite.apache.org>
>>> Subject: Re: [QUESTION] Contract for SqlValidator#validate
>>>
>>> IMO this seems an unexpected side effect; ideally it should be corrected,
>>> at the very least it should be noted on the javadoc with a TODO.
>>>
>>> Ruben
>>>
>>>
>>> On Tue, Jan 20, 2026 at 9:23 PM Julian Hyde <jh...@gmail.com>
>> wrote:
>>>
>>>> I’d say the basic mission of SqlValidator is to detect user errors and
>>>> give good messages, and assign types.
>>>>
>>>> I hate that it sometimes mutates the tree. Do that as little as
>> possible.
>>>> If you can remove mutations, do it.
>>>>
>>>> Julian
>>>>
>>>>> On Jan 20, 2026, at 13:18, Sergey Nuyanzin <sn...@gmail.com>
>> wrote:
>>>>>
>>>>> Hi everyone
>>>>>
>>>>> I faced behavior which seems weird to me, however I would like to
>>>>> double check first.
>>>>>
>>>>> Normally we call
>>>>> SqlNode validatedNode = validator.validate(inputNode); //here
>>>>> validator is SqlValidatorImpl
>>>>> and everything is ok if we do not use inputNode (for whatever reason
>> )
>>>>>
>>>>> However if inputNode is used anywhere after validation then in some
>>>>> cases validate method makes this node invalid by adding illegal nodes
>>>>>
>>>>> From the javadoc for SqlValidator interface it is not clear if this
>>>>> method is allowed to mutate SqlNode making it invalid or not[1]
>>>>>
>>>>> Would be great to get better understanding whether it is expected
>>>>> behavior (may be javadoc should be updated then) or not
>>>>>
>>>>> P.S. Also I tweaked SqlValidatorTest in a way to make it run
>>>>> validator.validate(inputNode) 2 times on the same input: there are a
>>>>> few tests failed because of the things mentioned above
>>>>>
>>>>> [1]
>>>>
>> https://github.com/apache/calcite/blob/270ba649cfa80228d3f4f6635a08ab06de504399/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java#L139-L146
>>>>> --
>>>>> Best regards,
>>>>> Sergey
>>>>
>>
>>
>>
>> --
>> Best regards,
>> Sergey
>>
Re: [QUESTION] Contract for SqlValidator#validate
Posted by Sergey Nuyanzin <sn...@gmail.com>.
Thanks for your responses
I will try to invest time here in my spare time and fix the cases (also
will create jira issue for that)
Best regards,
Sergey
On Wed, Jan 21, 2026, 10:27 Sergey Nuyanzin <sn...@gmail.com> wrote:
> I agree that it might modify tree
> my point is: if the input sqlNode is valid, after calling validate it
> should stay valid (even if changed)
> right now for some use cases it becomes invalid
>
> On Wed, Jan 21, 2026 at 10:03 AM Mihai Budiu <mb...@gmail.com> wrote:
> >
> > The validator does modify the expression tree, for example to insert
> explicit casts during type coercion. There is no way around this. In
> general, type inference may also modify the expression tree.
> >
> > Mihai
> >
> > ________________________________
> > From: Ruben Q L <ru...@gmail.com>
> > Sent: Wednesday, January 21, 2026 1:00 AM
> > To: dev@calcite.apache.org <de...@calcite.apache.org>
> > Subject: Re: [QUESTION] Contract for SqlValidator#validate
> >
> > IMO this seems an unexpected side effect; ideally it should be corrected,
> > at the very least it should be noted on the javadoc with a TODO.
> >
> > Ruben
> >
> >
> > On Tue, Jan 20, 2026 at 9:23 PM Julian Hyde <jh...@gmail.com>
> wrote:
> >
> > > I’d say the basic mission of SqlValidator is to detect user errors and
> > > give good messages, and assign types.
> > >
> > > I hate that it sometimes mutates the tree. Do that as little as
> possible.
> > > If you can remove mutations, do it.
> > >
> > > Julian
> > >
> > > > On Jan 20, 2026, at 13:18, Sergey Nuyanzin <sn...@gmail.com>
> wrote:
> > > >
> > > > Hi everyone
> > > >
> > > > I faced behavior which seems weird to me, however I would like to
> > > > double check first.
> > > >
> > > > Normally we call
> > > > SqlNode validatedNode = validator.validate(inputNode); //here
> > > > validator is SqlValidatorImpl
> > > > and everything is ok if we do not use inputNode (for whatever reason
> )
> > > >
> > > > However if inputNode is used anywhere after validation then in some
> > > > cases validate method makes this node invalid by adding illegal nodes
> > > >
> > > > From the javadoc for SqlValidator interface it is not clear if this
> > > > method is allowed to mutate SqlNode making it invalid or not[1]
> > > >
> > > > Would be great to get better understanding whether it is expected
> > > > behavior (may be javadoc should be updated then) or not
> > > >
> > > > P.S. Also I tweaked SqlValidatorTest in a way to make it run
> > > > validator.validate(inputNode) 2 times on the same input: there are a
> > > > few tests failed because of the things mentioned above
> > > >
> > > > [1]
> > >
> https://github.com/apache/calcite/blob/270ba649cfa80228d3f4f6635a08ab06de504399/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java#L139-L146
> > > > --
> > > > Best regards,
> > > > Sergey
> > >
>
>
>
> --
> Best regards,
> Sergey
>
Re: [QUESTION] Contract for SqlValidator#validate
Posted by Sergey Nuyanzin <sn...@gmail.com>.
I agree that it might modify tree
my point is: if the input sqlNode is valid, after calling validate it
should stay valid (even if changed)
right now for some use cases it becomes invalid
On Wed, Jan 21, 2026 at 10:03 AM Mihai Budiu <mb...@gmail.com> wrote:
>
> The validator does modify the expression tree, for example to insert explicit casts during type coercion. There is no way around this. In general, type inference may also modify the expression tree.
>
> Mihai
>
> ________________________________
> From: Ruben Q L <ru...@gmail.com>
> Sent: Wednesday, January 21, 2026 1:00 AM
> To: dev@calcite.apache.org <de...@calcite.apache.org>
> Subject: Re: [QUESTION] Contract for SqlValidator#validate
>
> IMO this seems an unexpected side effect; ideally it should be corrected,
> at the very least it should be noted on the javadoc with a TODO.
>
> Ruben
>
>
> On Tue, Jan 20, 2026 at 9:23 PM Julian Hyde <jh...@gmail.com> wrote:
>
> > I’d say the basic mission of SqlValidator is to detect user errors and
> > give good messages, and assign types.
> >
> > I hate that it sometimes mutates the tree. Do that as little as possible.
> > If you can remove mutations, do it.
> >
> > Julian
> >
> > > On Jan 20, 2026, at 13:18, Sergey Nuyanzin <sn...@gmail.com> wrote:
> > >
> > > Hi everyone
> > >
> > > I faced behavior which seems weird to me, however I would like to
> > > double check first.
> > >
> > > Normally we call
> > > SqlNode validatedNode = validator.validate(inputNode); //here
> > > validator is SqlValidatorImpl
> > > and everything is ok if we do not use inputNode (for whatever reason )
> > >
> > > However if inputNode is used anywhere after validation then in some
> > > cases validate method makes this node invalid by adding illegal nodes
> > >
> > > From the javadoc for SqlValidator interface it is not clear if this
> > > method is allowed to mutate SqlNode making it invalid or not[1]
> > >
> > > Would be great to get better understanding whether it is expected
> > > behavior (may be javadoc should be updated then) or not
> > >
> > > P.S. Also I tweaked SqlValidatorTest in a way to make it run
> > > validator.validate(inputNode) 2 times on the same input: there are a
> > > few tests failed because of the things mentioned above
> > >
> > > [1]
> > https://github.com/apache/calcite/blob/270ba649cfa80228d3f4f6635a08ab06de504399/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java#L139-L146
> > > --
> > > Best regards,
> > > Sergey
> >
--
Best regards,
Sergey
Re: [QUESTION] Contract for SqlValidator#validate
Posted by Mihai Budiu <mb...@gmail.com>.
The validator does modify the expression tree, for example to insert explicit casts during type coercion. There is no way around this. In general, type inference may also modify the expression tree.
Mihai
________________________________
From: Ruben Q L <ru...@gmail.com>
Sent: Wednesday, January 21, 2026 1:00 AM
To: dev@calcite.apache.org <de...@calcite.apache.org>
Subject: Re: [QUESTION] Contract for SqlValidator#validate
IMO this seems an unexpected side effect; ideally it should be corrected,
at the very least it should be noted on the javadoc with a TODO.
Ruben
On Tue, Jan 20, 2026 at 9:23 PM Julian Hyde <jh...@gmail.com> wrote:
> I’d say the basic mission of SqlValidator is to detect user errors and
> give good messages, and assign types.
>
> I hate that it sometimes mutates the tree. Do that as little as possible.
> If you can remove mutations, do it.
>
> Julian
>
> > On Jan 20, 2026, at 13:18, Sergey Nuyanzin <sn...@gmail.com> wrote:
> >
> > Hi everyone
> >
> > I faced behavior which seems weird to me, however I would like to
> > double check first.
> >
> > Normally we call
> > SqlNode validatedNode = validator.validate(inputNode); //here
> > validator is SqlValidatorImpl
> > and everything is ok if we do not use inputNode (for whatever reason )
> >
> > However if inputNode is used anywhere after validation then in some
> > cases validate method makes this node invalid by adding illegal nodes
> >
> > From the javadoc for SqlValidator interface it is not clear if this
> > method is allowed to mutate SqlNode making it invalid or not[1]
> >
> > Would be great to get better understanding whether it is expected
> > behavior (may be javadoc should be updated then) or not
> >
> > P.S. Also I tweaked SqlValidatorTest in a way to make it run
> > validator.validate(inputNode) 2 times on the same input: there are a
> > few tests failed because of the things mentioned above
> >
> > [1]
> https://github.com/apache/calcite/blob/270ba649cfa80228d3f4f6635a08ab06de504399/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java#L139-L146
> > --
> > Best regards,
> > Sergey
>
Re: [QUESTION] Contract for SqlValidator#validate
Posted by Ruben Q L <ru...@gmail.com>.
IMO this seems an unexpected side effect; ideally it should be corrected,
at the very least it should be noted on the javadoc with a TODO.
Ruben
On Tue, Jan 20, 2026 at 9:23 PM Julian Hyde <jh...@gmail.com> wrote:
> I’d say the basic mission of SqlValidator is to detect user errors and
> give good messages, and assign types.
>
> I hate that it sometimes mutates the tree. Do that as little as possible.
> If you can remove mutations, do it.
>
> Julian
>
> > On Jan 20, 2026, at 13:18, Sergey Nuyanzin <sn...@gmail.com> wrote:
> >
> > Hi everyone
> >
> > I faced behavior which seems weird to me, however I would like to
> > double check first.
> >
> > Normally we call
> > SqlNode validatedNode = validator.validate(inputNode); //here
> > validator is SqlValidatorImpl
> > and everything is ok if we do not use inputNode (for whatever reason )
> >
> > However if inputNode is used anywhere after validation then in some
> > cases validate method makes this node invalid by adding illegal nodes
> >
> > From the javadoc for SqlValidator interface it is not clear if this
> > method is allowed to mutate SqlNode making it invalid or not[1]
> >
> > Would be great to get better understanding whether it is expected
> > behavior (may be javadoc should be updated then) or not
> >
> > P.S. Also I tweaked SqlValidatorTest in a way to make it run
> > validator.validate(inputNode) 2 times on the same input: there are a
> > few tests failed because of the things mentioned above
> >
> > [1]
> https://github.com/apache/calcite/blob/270ba649cfa80228d3f4f6635a08ab06de504399/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java#L139-L146
> > --
> > Best regards,
> > Sergey
>
Re: [QUESTION] Contract for SqlValidator#validate
Posted by Julian Hyde <jh...@gmail.com>.
I’d say the basic mission of SqlValidator is to detect user errors and give good messages, and assign types.
I hate that it sometimes mutates the tree. Do that as little as possible. If you can remove mutations, do it.
Julian
> On Jan 20, 2026, at 13:18, Sergey Nuyanzin <sn...@gmail.com> wrote:
>
> Hi everyone
>
> I faced behavior which seems weird to me, however I would like to
> double check first.
>
> Normally we call
> SqlNode validatedNode = validator.validate(inputNode); //here
> validator is SqlValidatorImpl
> and everything is ok if we do not use inputNode (for whatever reason )
>
> However if inputNode is used anywhere after validation then in some
> cases validate method makes this node invalid by adding illegal nodes
>
> From the javadoc for SqlValidator interface it is not clear if this
> method is allowed to mutate SqlNode making it invalid or not[1]
>
> Would be great to get better understanding whether it is expected
> behavior (may be javadoc should be updated then) or not
>
> P.S. Also I tweaked SqlValidatorTest in a way to make it run
> validator.validate(inputNode) 2 times on the same input: there are a
> few tests failed because of the things mentioned above
>
> [1] https://github.com/apache/calcite/blob/270ba649cfa80228d3f4f6635a08ab06de504399/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java#L139-L146
> --
> Best regards,
> Sergey