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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>> Sent: Wednesday, January 21, 2026 1:00 AM >>> To: [email protected] <[email protected]> >>> 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 <[email protected]> >> 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 <[email protected]> >> 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 >>
