julianhyde commented on code in PR #3393:
URL: https://github.com/apache/calcite/pull/3393#discussion_r1306115154
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -4792,6 +4828,35 @@ private void convertValuesImpl(
true);
}
+ private int[] processColumnsMapping(RelDataType targetRowType, RelOptTable
targetTable) {
+ List<ColumnStrategy> strategies = targetTable.getColumnStrategies();
+ List<String> targetFields = targetRowType.getFieldNames();
+ List<RelDataTypeField> tblFields = targetTable.getRowType().getFieldList();
+
+ int[] mapping = new int[targetFields.size()];
+
+ Arrays.fill(mapping, -1);
+
+ Ord.forEach(tblFields, (fld, i) -> {
+ if (strategies.get(i) == ColumnStrategy.DEFAULT) {
+ int pos = targetFields.indexOf(fld.getName());
+ if (pos != -1) {
+ mapping[pos] = i;
+ }
+ }
+ });
+
+ if (this.getClass().desiredAssertionStatus()) {
+ long items = Arrays.stream(mapping).count();
Review Comment:
why not just `mapping.length`?
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -4792,6 +4828,35 @@ private void convertValuesImpl(
true);
}
+ private int[] processColumnsMapping(RelDataType targetRowType, RelOptTable
targetTable) {
+ List<ColumnStrategy> strategies = targetTable.getColumnStrategies();
+ List<String> targetFields = targetRowType.getFieldNames();
+ List<RelDataTypeField> tblFields = targetTable.getRowType().getFieldList();
+
+ int[] mapping = new int[targetFields.size()];
+
+ Arrays.fill(mapping, -1);
+
+ Ord.forEach(tblFields, (fld, i) -> {
+ if (strategies.get(i) == ColumnStrategy.DEFAULT) {
+ int pos = targetFields.indexOf(fld.getName());
+ if (pos != -1) {
+ mapping[pos] = i;
+ }
+ }
+ });
+
+ if (this.getClass().desiredAssertionStatus()) {
Review Comment:
I'd remove this `if`. This is not performance-critical code. The assertion
is worth checking even if assertions are disabled.
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -4769,21 +4779,47 @@ private void convertValuesImpl(
return;
}
- for (SqlNode rowConstructor1 : values.getOperandList()) {
- SqlCall rowConstructor = (SqlCall) rowConstructor1;
+ SqlCall insertOp = callStack.peek();
+ boolean processDefaults = insertOp != null && DefaultChecker.check(values);
+
+ if (targetRowType == null) {
+ RelDataType listType = validator().getValidatedNodeType(values);
+ targetRowType = SqlTypeUtil.promoteToRowType(typeFactory, listType,
null);
+ }
+
+ assert insertOp instanceof SqlInsert || !processDefaults : "operation: " +
insertOp;
+
+ @Nullable InitializerExpressionFactory initializerFactory = processDefaults
+ ? getInitializerFactory(getNamespace(requireNonNull(insertOp,
"insertOp")).getTable())
+ : null;
+ @Nullable RelOptTable targetTable = processDefaults
+ ? getTargetTable(requireNonNull(insertOp, "insertOp")) : null;
+ int[] mapping = processDefaults
+ ? processColumnsMapping(requireNonNull(targetRowType, "targetRowType"),
+ requireNonNull(targetTable, "targetTable")) : null;
Review Comment:
This code block is hard to read. All 3 variables have `processDefaults ?` in
their expression, and your inconsistent formatting doesn't help. One `if
(processDefaults)` block would be easier to read.
##########
server/src/test/java/org/apache/calcite/test/ServerParserTest.java:
##########
@@ -56,6 +56,11 @@ class ServerParserTest extends SqlParserTest {
.ok("CREATE SCHEMA `X`");
}
+ @Test void testProcessCreateTableWithDefaul() {
Review Comment:
s/Defaul/Default/
##########
testkit/src/main/java/org/apache/calcite/test/QuidemTest.java:
##########
@@ -118,7 +118,6 @@ public abstract class QuidemTest {
@SuppressWarnings({"BetaApi", "UnstableApiUsage"})
protected static Collection<String> data(String first) {
- // inUrl = "file:/home/fred/calcite/core/target/test-classes/sql/agg.iq"
Review Comment:
why delete this comment?
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -281,6 +285,8 @@ public class SqlToRelConverter {
*/
private final Deque<String> datasetStack = new ArrayDeque<>();
+ private final Deque<SqlCall> callStack = new ArrayDeque<>();
Review Comment:
needs javadoc
--
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]
