kennknowles commented on a change in pull request #12596:
URL: https://github.com/apache/beam/pull/12596#discussion_r487239063
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/SqlOperators.java
##########
@@ -132,6 +134,8 @@
public static final SqlOperator DATE_OP =
createUdfOperator("DATE", BeamBuiltinMethods.DATE_METHOD);
+ // TODO: Fix Later
Review comment:
Same here, link to an issue so someone who finds the TODO can see if
anyone else is working on it or if there is some history.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -218,6 +218,8 @@ SimpleCatalog createPopulatedCatalog(
return catalog;
}
+ // TODO: Fix Later
Review comment:
Link to BEAM-10498 and leave it open?
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
##########
@@ -187,6 +189,8 @@ private BeamRelNode convertToBeamRelInternal(String sql,
QueryParameters queryPa
return (BeamRelNode) plannerImpl.transform(0, desiredTraits, root.rel);
}
+ // TODO: Resolve later
+ @SuppressWarnings("nullness")
Review comment:
I'm guessing this is just a Calcite API thing and we have to suppress
warnings because we do not have annotated Calcite? So it is OK to suppress but
try to get the scope of suppression as local as possible.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/SqlOperators.java
##########
@@ -132,6 +134,8 @@
public static final SqlOperator DATE_OP =
createUdfOperator("DATE", BeamBuiltinMethods.DATE_METHOD);
+ // TODO: Fix Later
Review comment:
Same here, link to an issue so someone who finds the TODO can see if
anyone else is working on it or if there is some history.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -218,6 +218,8 @@ SimpleCatalog createPopulatedCatalog(
return catalog;
}
+ // TODO: Fix Later
Review comment:
Link to BEAM-10498 and leave it open?
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
##########
@@ -187,6 +189,8 @@ private BeamRelNode convertToBeamRelInternal(String sql,
QueryParameters queryPa
return (BeamRelNode) plannerImpl.transform(0, desiredTraits, root.rel);
}
+ // TODO: Resolve later
+ @SuppressWarnings("nullness")
Review comment:
I'm guessing this is just a Calcite API thing and we have to suppress
warnings because we do not have annotated Calcite? So it is OK to suppress but
try to get the scope of suppression as local as possible.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/SqlOperators.java
##########
@@ -132,6 +134,8 @@
public static final SqlOperator DATE_OP =
createUdfOperator("DATE", BeamBuiltinMethods.DATE_METHOD);
+ // TODO: Fix Later
Review comment:
Same here, link to an issue so someone who finds the TODO can see if
anyone else is working on it or if there is some history.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -218,6 +218,8 @@ SimpleCatalog createPopulatedCatalog(
return catalog;
}
+ // TODO: Fix Later
Review comment:
Link to BEAM-10498 and leave it open?
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
##########
@@ -187,6 +189,8 @@ private BeamRelNode convertToBeamRelInternal(String sql,
QueryParameters queryPa
return (BeamRelNode) plannerImpl.transform(0, desiredTraits, root.rel);
}
+ // TODO: Resolve later
+ @SuppressWarnings("nullness")
Review comment:
I'm guessing this is just a Calcite API thing and we have to suppress
warnings because we do not have annotated Calcite? So it is OK to suppress but
try to get the scope of suppression as local as possible.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/SqlOperators.java
##########
@@ -132,6 +134,8 @@
public static final SqlOperator DATE_OP =
createUdfOperator("DATE", BeamBuiltinMethods.DATE_METHOD);
+ // TODO: Fix Later
Review comment:
Same here, link to an issue so someone who finds the TODO can see if
anyone else is working on it or if there is some history.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -218,6 +218,8 @@ SimpleCatalog createPopulatedCatalog(
return catalog;
}
+ // TODO: Fix Later
Review comment:
Link to BEAM-10498 and leave it open?
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
##########
@@ -187,6 +189,8 @@ private BeamRelNode convertToBeamRelInternal(String sql,
QueryParameters queryPa
return (BeamRelNode) plannerImpl.transform(0, desiredTraits, root.rel);
}
+ // TODO: Resolve later
+ @SuppressWarnings("nullness")
Review comment:
I'm guessing this is just a Calcite API thing and we have to suppress
warnings because we do not have annotated Calcite? So it is OK to suppress but
try to get the scope of suppression as local as possible.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/SqlOperators.java
##########
@@ -132,6 +134,8 @@
public static final SqlOperator DATE_OP =
createUdfOperator("DATE", BeamBuiltinMethods.DATE_METHOD);
+ // TODO: Fix Later
Review comment:
Same here, link to an issue so someone who finds the TODO can see if
anyone else is working on it or if there is some history.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -218,6 +218,8 @@ SimpleCatalog createPopulatedCatalog(
return catalog;
}
+ // TODO: Fix Later
Review comment:
Link to BEAM-10498 and leave it open?
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
##########
@@ -187,6 +189,8 @@ private BeamRelNode convertToBeamRelInternal(String sql,
QueryParameters queryPa
return (BeamRelNode) plannerImpl.transform(0, desiredTraits, root.rel);
}
+ // TODO: Resolve later
+ @SuppressWarnings("nullness")
Review comment:
I'm guessing this is just a Calcite API thing and we have to suppress
warnings because we do not have annotated Calcite? So it is OK to suppress but
try to get the scope of suppression as local as possible.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/SqlOperators.java
##########
@@ -132,6 +134,8 @@
public static final SqlOperator DATE_OP =
createUdfOperator("DATE", BeamBuiltinMethods.DATE_METHOD);
+ // TODO: Fix Later
Review comment:
Same here, link to an issue so someone who finds the TODO can see if
anyone else is working on it or if there is some history.
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -218,6 +218,8 @@ SimpleCatalog createPopulatedCatalog(
return catalog;
}
+ // TODO: Fix Later
Review comment:
Link to BEAM-10498 and leave it open?
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
##########
@@ -187,6 +189,8 @@ private BeamRelNode convertToBeamRelInternal(String sql,
QueryParameters queryPa
return (BeamRelNode) plannerImpl.transform(0, desiredTraits, root.rel);
}
+ // TODO: Resolve later
+ @SuppressWarnings("nullness")
Review comment:
I'm guessing this is just a Calcite API thing and we have to suppress
warnings because we do not have annotated Calcite? So it is OK to suppress but
try to get the scope of suppression as local as possible.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
