sbroeder commented on code in PR #5399:
URL: https://github.com/apache/calcite/pull/5399#discussion_r4517637176
##########
core/src/main/java/org/apache/calcite/sql/validate/AbstractNamespace.java:
##########
@@ -89,15 +101,22 @@ abstract class AbstractNamespace implements
SqlValidatorNamespace {
@Override public final void validate(RelDataType targetRowType) {
switch (status) {
case UNVALIDATED:
+ status = SqlValidatorImpl.Status.IN_PROGRESS;
try {
- status = SqlValidatorImpl.Status.IN_PROGRESS;
checkArgument(rowType == null,
"Namespace.rowType must be null before validate has been called");
RelDataType type = validateImpl(targetRowType);
requireNonNull(type, "validateImpl() returned null");
setType(type);
- } finally {
status = SqlValidatorImpl.Status.VALID;
+ } catch (RuntimeException e) {
+ // Mark VALID so later calls do not re-enter validateImpl(),
+ // which may have already partially mutated shared validator state.
+ // Store the exception so getRowType() re-throws the real error
+ // instead of recursing into a null rowType.
+ status = SqlValidatorImpl.Status.VALID;
Review Comment:
> clearly, this enum is misnamed, it should be called COMPLETED, as the
javadoc indicates. But I don't know if this should be part of this PR.
Agreed, COMPLETED describes it better, and the Javadoc already says so. I'd
prefer to keep this PR to the bug fix, since Status is a public enum and
renaming it is an API change. I'm happy to file a follow-up JIRA for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
