mihaibudiu commented on code in PR #4939:
URL: https://github.com/apache/calcite/pull/4939#discussion_r3236210964
##########
babel/src/main/codegen/config.fmpp:
##########
@@ -618,6 +618,7 @@ data: {
includeParsingStringLiteralAsArrayLiteral: true
includeIntervalWithoutQualifier: true
includeStarExclude: true
+ includeStarReplace: true
Review Comment:
I wonder whether this should reuse the same flag as StarExclude.
Too many flags are harder to manage.
##########
babel/src/test/java/org/apache/calcite/test/BabelTest.java:
##########
@@ -275,6 +275,61 @@ names, is(
.fails("SELECT \\* EXCLUDE/EXCEPT list cannot exclude all columns");
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-7532";>
+ * [CALCITE-7532] Support the syntax SELECT * REPLACE(expr as column)</a>.
+ * */
+ @Test void testStarReplaceValidation() {
+ final SqlValidatorFixture fixture = Fixtures.forValidator()
+ .withParserConfig(p ->
p.withParserFactory(SqlBabelParserImpl.FACTORY));
+
+ fixture.withSql("select * replace(empno + 1 as empno) from emp")
+ .type(type -> {
+ final List<String> names = type.getFieldList().stream()
+ .map(RelDataTypeField::getName)
+ .collect(Collectors.toList());
+ assertThat(
+ names, is(
+ ImmutableList.of("EMPNO", "ENAME", "JOB", "MGR",
+ "HIREDATE", "SAL", "COMM", "DEPTNO", "SLACKER")));
+ // Verify that EMPNO type is still INTEGER (or similar)
Review Comment:
how about one test where the new column has a completely different type?
I assume that's possible
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -661,8 +672,35 @@ private boolean expandStar(List<SqlNode> selectItems,
Set<String> aliases,
final List<SqlIdentifier> excludeIdentifiers =
excludeList == null ? Collections.emptyList() :
extractExcludeIdentifiers(excludeList);
final boolean[] excludeMatched = new boolean[excludeIdentifiers.size()];
+ final Map<String, SqlNode> replaceMap =
Review Comment:
it looks like you can just call extractReplaceMap, it checks for null itself
##########
babel/src/test/java/org/apache/calcite/test/BabelTest.java:
##########
@@ -275,6 +275,61 @@ names, is(
.fails("SELECT \\* EXCLUDE/EXCEPT list cannot exclude all columns");
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-7532";>
+ * [CALCITE-7532] Support the syntax SELECT * REPLACE(expr as column)</a>.
+ * */
+ @Test void testStarReplaceValidation() {
+ final SqlValidatorFixture fixture = Fixtures.forValidator()
+ .withParserConfig(p ->
p.withParserFactory(SqlBabelParserImpl.FACTORY));
+
+ fixture.withSql("select * replace(empno + 1 as empno) from emp")
+ .type(type -> {
+ final List<String> names = type.getFieldList().stream()
+ .map(RelDataTypeField::getName)
+ .collect(Collectors.toList());
+ assertThat(
+ names, is(
+ ImmutableList.of("EMPNO", "ENAME", "JOB", "MGR",
+ "HIREDATE", "SAL", "COMM", "DEPTNO", "SLACKER")));
+ // Verify that EMPNO type is still INTEGER (or similar)
+
assertThat(type.getFieldList().get(0).getType().getSqlTypeName().getName(),
+ is("INTEGER"));
+ });
+
+ fixture.withSql("select * replace(empno + 1 as empno, sal * 2 as sal) from
emp")
+ .type(type -> {
+ final List<String> names = type.getFieldList().stream()
+ .map(RelDataTypeField::getName)
+ .collect(Collectors.toList());
+ assertThat(
+ names, is(
+ ImmutableList.of("EMPNO", "ENAME", "JOB", "MGR",
+ "HIREDATE", "SAL", "COMM", "DEPTNO", "SLACKER")));
+ });
+
+ // Unknown column in REPLACE list
+ fixture.withSql("select * replace(empno + 1 as ^foo^) from emp")
+ .fails("SELECT \\* REPLACE list contains unknown column\\(s\\): FOO");
+
+ // Table-qualified star with REPLACE
+ fixture.withSql("select e.* replace(e.empno + 1 as e.empno)"
+ + " from emp e join dept d on e.deptno = d.deptno")
+ .type(type -> {
+ final List<String> names = type.getFieldList().stream()
+ .map(RelDataTypeField::getName)
+ .collect(Collectors.toList());
+ assertThat(
+ names, is(
+ ImmutableList.of("EMPNO", "ENAME", "JOB", "MGR",
+ "HIREDATE", "SAL", "COMM", "DEPTNO", "SLACKER")));
+ });
+
+ // REPLACE with unknown qualified column
+ fixture.withSql("select e.* replace(e.empno + 1 as ^d.deptno^)"
Review Comment:
how about a test where the same column is replaced twice?
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -903,6 +986,82 @@ private void
throwIfExcludeEliminatesAllColumns(List<SqlIdentifier> excludeIdent
}
}
+ private static Map<String, SqlNode> extractReplaceMap(@Nullable SqlNodeList
replaceList) {
+ if (replaceList == null) {
+ return ImmutableMap.of();
+ }
+ final ImmutableMap.Builder<String, SqlNode> builder =
ImmutableMap.builder();
+ for (SqlNode node : replaceList) {
+ if (node instanceof SqlCall) {
Review Comment:
what if it's not a SqlCall?
Some missing assertion?
##########
site/_docs/reference.md:
##########
@@ -244,9 +244,14 @@ starWithExclude:
*
| * EXCLUDE '(' column [, column ]* ')'
+starWithReplace:
+ *
+ | * REPLACE '(' expression AS column [, expression AS column ]* ')'
+
Note:
* `SELECT * EXCLUDE (...)` is recognized only when the Babel parser is
enabled. It sets the generated parser configuration flag `includeStarExclude`
to `true` (the standard parser leaves that flag `false`), which allows a `STAR`
token followed by `EXCLUDE` (or the alias `EXCEPT`) and a parenthesized
identifier list to be parsed into a `SqlStarExclude` node and ensures
validators respect the exclusion list when expanding the projection. Reusing
the same parser configuration elsewhere enables the same syntax for other
components that need it.
+* `SELECT * REPLACE (...)` is recognized only when the Babel parser is
enabled. It sets the generated parser configuration flag `includeStarReplace`
to `true` (the standard parser leaves that flag `false`), which allows a `STAR`
token followed by `REPLACE` and a parenthesized list of `expression AS column`
pairs to be parsed into a `SqlStarReplace` node and ensures validators
substitute the specified expressions for the matching columns when expanding
the projection. The column alias must either be a simple identifier or, for a
table-qualified star such as `t.*`, a qualified identifier whose prefix matches
the star's table alias. Reusing the same parser configuration elsewhere enables
the same syntax for other components that need it.
Review Comment:
I don't understand exactly the last sentence. Does it even belong in the
reference? Seems to be more useful for developers.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -781,6 +841,28 @@ private boolean expandStar(List<SqlNode> selectItems,
Set<String> aliases,
if (shouldExcludeField(excludeList, columnId, resolvedNameMatcher)) {
continue;
}
+ final SqlNode replacement =
+ findReplacement(columnName, replaceMap, resolvedNameMatcher);
+ if (replacement != null) {
Review Comment:
if this check fails is there an error thrown later?
Then please document this after the if
--
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]
