[CALCITE-7310] Support the syntax SELECT * EXCLUDE(columns) #4662

xiedeyantu · 2025-12-03T10:44:55Z

See CALCITE-7310

mihaibudiu

That was fast!

mihaibudiu · 2025-12-03T16:52:53Z

Let's see if there are any other comments.

mihaibudiu · 2025-12-03T16:54:25Z

Actually, the documentation needs to be updated: reference.md

mihaibudiu · 2025-12-03T17:56:00Z

One more thing: can you add some tests with the syntax T.* EXCEPT(...)? Will this work in a join with both inputs?

xiedeyantu · 2025-12-03T22:29:46Z

One more thing: can you add some tests with the syntax T.* EXCEPT(...)? Will this work in a join with both inputs?

This case is not supported yet. I will look into how we can add support for it.

xiedeyantu · 2025-12-03T23:49:00Z

I think supporting T.* EXCLUDE(...) will require some additional work. We should probably remove the LGTM label for now, as I may need to do some refactoring.

mihaibudiu

Let's see if this can include the T.* syntax as well.

xiedeyantu · 2025-12-04T00:21:12Z

Let's see if this can include the T.* syntax as well.

Maybe yes, I will check it.

babel/src/test/resources/sql/select.iq

mihaibudiu

Having the changes in a separate commit would have made this a bit easier to review

mihaibudiu · 2025-12-04T22:04:18Z

site/_docs/reference.md

  * A scalar value has length 1;
  * The length of array or object is the number of elements is contains.

+### Babel parser extensions


could you instead add the new rule to the grammar and make this a note after the grammar rule is described?

mihaibudiu · 2025-12-04T22:05:43Z

core/src/main/java/org/apache/calcite/sql/SqlStarExclude.java

+import static java.util.Objects.requireNonNull;
+
+/**
+ * Represents {@code SELECT * EXCLUDE(...)} when parsed by the Babel parser.


The fact that the Babel parser (only) uses this class should probably not be part of the Javadoc of this class.

mihaibudiu · 2025-12-04T22:06:51Z

core/src/main/java/org/apache/calcite/sql/SqlStarExclude.java

+  }
+
+  @Override public void validate(SqlValidator validator, SqlValidatorScope scope) {
+    // Babel already validates the exclude list


This seems odd, what if someone wants to use this outside of Babel?

Deleted this empty override of the validate method.

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

mihaibudiu · 2025-12-04T22:12:57Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java


+  private static boolean matchesExcludeNames(List<String> columnNames,
+      List<String> excludeNames, SqlNameMatcher nameMatcher) {
+    if (excludeNames.size() > columnNames.size()) {


is this right even if there are duplicates?
or have duplicates been checked elsewhere?

The EXCLUDE list is compared item by item, and duplicates are also included. Additionally, there is a Boolean type for identification, so no extra consideration for duplicates is needed here.

here you are returning after comparing only lengths

Sorry, this should be a comparison between column names and fully qualified names. For example, [deptno] (in excludeNames) and [emp.deptno] (in columnNames).

So each list is a qualified identifier?

Yes, excludeNamescan be either [deptno] or [emp, deptno], while columnNameswould be [emp, deptno] or [emp, empno].

please make this clearer in the name of the arguments

Addressed in cb1be43

mihaibudiu · 2025-12-04T22:13:35Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+      return false;
+    }
+    for (SqlNode node : excludeList) {
+      if (!(node instanceof SqlIdentifier)) {


I suspect this can never happen, so this can perhaps be an assertion?

xiedeyantu · 2025-12-04T22:28:43Z

Having the changes in a separate commit would have made this a bit easier to review

Yes, because it was basically a major refactoring, I force-pushed it. Then do you think it's better to let the default parser have this capability without distinguishing Babel separately?

mihaibudiu · 2025-12-04T22:30:23Z

No, I think it's fine to have it in Babel, but in general I don't think a class should list in JavaDoc the places where it's used unless the class is private. After all, you don't know what other code people will write in the future using this class.

xiedeyantu · 2025-12-06T00:21:07Z

Some other comments have been addressed. If I missed anything, please let me know.

dssysolyatin · 2025-12-06T11:56:13Z

core/src/main/java/org/apache/calcite/sql/SqlStarExclude.java

+ * Represents {@code SELECT * EXCLUDE(...)}.
+ */
+public class SqlStarExclude extends SqlCall {
+  public static final SqlSpecialOperator OPERATOR =


Change
public static final SqlSpecialOperator OPERATOR
to
public static final SqlOperator OPERATOR.

Also, implement the createCall method for the operator

Thanks for review, I have addressed this in 6f38e07

dssysolyatin · 2025-12-06T12:12:24Z

core/src/main/java/org/apache/calcite/sql/SqlStarExclude.java

+    return ImmutableNullableList.of(starIdentifier, excludeList);
+  }
+
+  @Override public SqlNode clone(SqlParserPos pos) {


There is no need to override clone if operator createCall is implemented correctly

dssysolyatin

Looks good. I added a few minor comments

dssysolyatin · 2025-12-06T12:15:29Z

core/src/main/java/org/apache/calcite/sql/SqlStarExclude.java

+  }
+
+  @Override public List<SqlNode> getOperandList() {
+    return ImmutableNullableList.of(starIdentifier, excludeList);


Why it is ImmutableNullableList and not just ImmutableList ? If I understand correctly both starIdentifier and excludeList are not nullable.

dssysolyatin · 2025-12-06T12:18:20Z

core/src/main/java/org/apache/calcite/sql/SqlStarExclude.java

+    starIdentifier.unparse(writer, leftPrec, rightPrec);
+    writer.sep("EXCLUDE");
+    writer.print("(");
+    for (int i = 0; i < excludeList.size(); i++) {


Why do you need your own unparse logic for SqlNodeList instead of using excludeList.unparse

final SqlWriter.Frame frame = writer.startList("(", ")"); excludeList.unparse(...) writer.endList(frame)

xiedeyantu · 2025-12-06T13:15:30Z

I've made some adjustments to SqlStarExclude based on your comments. Please review it again. Thank you @dssysolyatin .

sonarqubecloud · 2025-12-06T13:25:45Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mihaibudiu

This looks great from my pov, if @dssysolyatin likes it we can merge it after squashing.

dssysolyatin

Looks good. Feel free to merge

mihaibudiu · 2025-12-08T18:26:30Z

@xiedeyantu please squash the commits; feel free to merge if you want, otherwise I will do it

xiedeyantu · 2025-12-09T01:12:19Z

Jenkins might have some issues. Should we wait for it?

mihaibudiu · 2025-12-09T01:38:59Z

Nah, jenkins can run later too

mihaibudiu approved these changes Dec 3, 2025

View reviewed changes

mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Dec 3, 2025

mihaibudiu mentioned this pull request Dec 3, 2025

[SQL] Support for SELECT * EXCLUDE cols feldera/feldera#5216

Open

mihaibudiu removed the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Dec 4, 2025

mihaibudiu reviewed Dec 4, 2025

View reviewed changes

xiedeyantu force-pushed the CALCITE-7310 branch 3 times, most recently from 63ea502 to 155496e Compare December 4, 2025 04:55

xiedeyantu commented Dec 4, 2025

View reviewed changes

babel/src/test/resources/sql/select.iq Show resolved Hide resolved

mihaibudiu reviewed Dec 4, 2025

View reviewed changes

mihaibudiu mentioned this pull request Dec 4, 2025

[CALCITE-7311] Support the syntax ROW(*) to create a nested ROW type with all columns #4665

Draft

dssysolyatin reviewed Dec 6, 2025

View reviewed changes

xiedeyantu force-pushed the CALCITE-7310 branch from bceaf40 to 0b84851 Compare December 6, 2025 13:00

xiedeyantu requested a review from dssysolyatin December 7, 2025 20:44

mihaibudiu approved these changes Dec 8, 2025

View reviewed changes

mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Dec 8, 2025

dssysolyatin approved these changes Dec 8, 2025

View reviewed changes

[CALCITE-7310] Support the syntax SELECT * EXCLUDE(columns)

7783877

xiedeyantu force-pushed the CALCITE-7310 branch from 0b84851 to 7783877 Compare December 8, 2025 22:43

xiedeyantu merged commit 935d676 into apache:main Dec 9, 2025
18 of 19 checks passed

[CALCITE-7310] Support the syntax SELECT * EXCLUDE(columns) #4662

[CALCITE-7310] Support the syntax SELECT * EXCLUDE(columns) #4662

Uh oh!

Conversation

xiedeyantu commented Dec 3, 2025

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

mihaibudiu commented Dec 3, 2025

Uh oh!

mihaibudiu commented Dec 3, 2025

Uh oh!

mihaibudiu commented Dec 3, 2025

Uh oh!

xiedeyantu commented Dec 3, 2025

Uh oh!

xiedeyantu commented Dec 3, 2025

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

xiedeyantu commented Dec 4, 2025

Uh oh!

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiedeyantu Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiedeyantu commented Dec 4, 2025

Uh oh!

mihaibudiu commented Dec 4, 2025

Uh oh!

xiedeyantu commented Dec 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dssysolyatin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiedeyantu commented Dec 6, 2025

Uh oh!

sonarqubecloud bot commented Dec 6, 2025

Quality Gate passed

xiedeyantu Dec 5, 2025 •

edited

Loading