mihaibudiu commented on code in PR #5399:
URL: https://github.com/apache/calcite/pull/5399#discussion_r4516397334
##########
core/src/main/java/org/apache/calcite/sql/advise/SqlAdvisorValidator.java:
##########
@@ -184,17 +182,54 @@ private void registerId(SqlIdentifier id,
SqlValidatorScope scope) {
}
}
+ /**
+ * Masks type-inference {@link CalciteException} during validation of
+ * sub-expressions so that the advisor can continue providing hints even
+ * when a namespace fails to validate, for example an unknown column in
+ * an incomplete SQL fragment.
+ */
+ @Override protected void inferUnknownTypes(
Review Comment:
Your issue title should probably make it clear that this is an advisor
problem.
##########
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.
--
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]
