[
https://issues.apache.org/jira/browse/CALCITE-7792?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sean Broeder updated CALCITE-7792:
----------------------------------
Description:
The finally block in AbstractNamespace.validate() sets the status to VALID even
when validateImpl() throws. That leaves rowType null. A later getRowType() on
the same namespace calls validateNamespace(), which does nothing because the
status is already VALID. validateNamespace() then calls getType(), which calls
getRowType() again. The result is a StackOverflowError in place of the original
error.
For example:
{code:java}
select * from (select bad_col from emp) as t
{code}
Validating this throws "Column 'BAD_COL' not found in any table" the first
time, as expected. But a second, independent getRowType() call on the inner
SELECT's namespace then throws StackOverflowError instead of re-surfacing that
error.
Ordinary SQL through a single validate() call does not hit this. It needs a
caller that catches the exception and keeps going, such as a validator
extension that probes a namespace and treats failure as "no match", or code
that asks for a namespace's type after a failed validation. SqlAdvisorValidator
is such a caller, because it validates incomplete SQL and must tolerate
namespaces that fail.
SqlAdvisorValidator catches CalciteException and continues, but it does not
overflow. Its activeNamespaces guard treats any repeat call to
validateNamespace() as a cycle and sets an empty struct type. That hides the
overflow, but it also gives wrong results.
For example:
{code:java}
select ename from (select ?, ename from emp)
{code}
returns no completion hints, where COLUMN(ENAME) is expected.
Setting the status back to UNVALIDATED on failure is not enough. validateImpl()
may already have changed shared validator state, so entering it a second time
is unsafe. Instead, store the exception and mark the namespace VALID. A later
getRowType() or getType() re-throws the original error.
In SqlAdvisorValidator:
- Remove the activeNamespaces guard. It treated any repeat call to
validateNamespace() as a cycle, but AbstractNamespace already handles repeat
calls through its own status field.
- validateNamespace() masks CalciteException and falls back to an empty struct
type.
- Override inferUnknownTypes() and checkRollUp() to swallow CalciteException,
so one unresolvable item doesn't abort validation of the whole enclosing
namespace. SqlValidatorImpl.checkRollUp() becomes protected for this.
The namespace's cycle check throws AssertionError, which the advisor does not
mask.
was:
The finally block in AbstractNamespace.validate() sets the status to VALID even
when validateImpl() throws. That leaves rowType null. A later getRowType() on
the same namespace calls validateNamespace(), which does nothing because the
status is already VALID. validateNamespace() then calls getType(), which calls
getRowType() again. The result is a StackOverflowError in place of the original
error.
For example:
{code:java}
select * from (select bad_col from emp) as t
{code}
Validating this throws "Column 'BAD_COL' not found in any table" the first
time, as expected. But a second, independent getRowType() call on the inner
SELECT's namespace then throws StackOverflowError instead of re-surfacing that
error.
Ordinary SQL through a single validate() call does not hit this. It needs a
caller that catches the exception and keeps going, such as a validator
extension that probes a namespace and treats failure as "no match", or code
that asks for a namespace's type after a failed validation. SqlAdvisorValidator
is such a caller, because it validates incomplete SQL and must tolerate
namespaces that fail.
SqlAdvisorValidator catches CalciteException and continues, but it does not
overflow. Its activeNamespaces guard treats any repeat call to
validateNamespace() as a cycle and sets an empty struct type. That hides the
overflow, but it also gives wrong results.
For example:
{code:java}
select ename from (select ?, ename from emp)
{code}
returns no completion hints, where COLUMN(ENAME) is expected.
Setting the status back to UNVALIDATED on failure is not enough. validateImpl()
may already have changed shared validator state, so entering it a second time
is unsafe. Instead, store the exception and mark the namespace VALID. A later
getRowType() or getType() re-throws the original error.
In SqlAdvisorValidator:
- Remove the activeNamespaces guard. It treated any repeat call to
validateNamespace() as a cycle, but AbstractNamespace already handles repeat
calls through its own status field.
- validateNamespace() masks CalciteException and falls back to an empty struct
type.
- Override inferUnknownTypes() and checkRollUp() to swallow CalciteException,
so one unresolvable item doesn't abort validation of the whole enclosing
namespace. SqlValidatorImpl.checkRollUp() becomes protected forthis.
The namespace's cycle check throws AssertionError, which the advisor does not
mask.
> Failed namespace validation leaves rowType null and causes stack overflow in
> getRowType()
> -----------------------------------------------------------------------------------------
>
> Key: CALCITE-7792
> URL: https://issues.apache.org/jira/browse/CALCITE-7792
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.42.0
> Reporter: Sean Broeder
> Assignee: Sean Broeder
> Priority: Major
> Labels: pull-request-available
>
> The finally block in AbstractNamespace.validate() sets the status to VALID
> even when validateImpl() throws. That leaves rowType null. A later
> getRowType() on the same namespace calls validateNamespace(), which does
> nothing because the status is already VALID. validateNamespace() then calls
> getType(), which calls getRowType() again. The result is a StackOverflowError
> in place of the original error.
> For example:
> {code:java}
> select * from (select bad_col from emp) as t
> {code}
> Validating this throws "Column 'BAD_COL' not found in any table" the first
> time, as expected. But a second, independent getRowType() call on the inner
> SELECT's namespace then throws StackOverflowError instead of re-surfacing
> that error.
> Ordinary SQL through a single validate() call does not hit this. It needs a
> caller that catches the exception and keeps going, such as a validator
> extension that probes a namespace and treats failure as "no match", or code
> that asks for a namespace's type after a failed validation.
> SqlAdvisorValidator is such a caller, because it validates incomplete SQL and
> must tolerate namespaces that fail.
> SqlAdvisorValidator catches CalciteException and continues, but it does not
> overflow. Its activeNamespaces guard treats any repeat call to
> validateNamespace() as a cycle and sets an empty struct type. That hides the
> overflow, but it also gives wrong results.
> For example:
> {code:java}
> select ename from (select ?, ename from emp)
> {code}
> returns no completion hints, where COLUMN(ENAME) is expected.
> Setting the status back to UNVALIDATED on failure is not enough.
> validateImpl() may already have changed shared validator state, so entering
> it a second time is unsafe. Instead, store the exception and mark the
> namespace VALID. A later getRowType() or getType() re-throws the original
> error.
> In SqlAdvisorValidator:
> - Remove the activeNamespaces guard. It treated any repeat call to
> validateNamespace() as a cycle, but AbstractNamespace already handles repeat
> calls through its own status field.
> - validateNamespace() masks CalciteException and falls back to an empty
> struct type.
> - Override inferUnknownTypes() and checkRollUp() to swallow
> CalciteException, so one unresolvable item doesn't abort validation of the
> whole enclosing namespace. SqlValidatorImpl.checkRollUp() becomes protected
> for this.
> The namespace's cycle check throws AssertionError, which the advisor does not
> mask.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
