github-actions[bot] commented on code in PR #66007:
URL: https://github.com/apache/doris/pull/66007#discussion_r3684718340
##########
fe/fe-type/src/main/java/org/apache/doris/catalog/Type.java:
##########
@@ -482,18 +486,32 @@ public String hideVersionForVersionColumn(Boolean
isToSql) {
}
return typeStr.toString();
} else if (isArrayType()) {
- String nestedDesc = ((ArrayType)
this).getItemType().hideVersionForVersionColumn(isToSql);
+ String nestedDesc = ((ArrayType) this).getItemType()
+ .hideVersionForVersionColumn(isToSql, showNestedComment);
return "array<" + nestedDesc + ">";
} else if (isMapType()) {
- String keyDesc = ((MapType)
this).getKeyType().hideVersionForVersionColumn(isToSql);
- String valueDesc = ((MapType)
this).getValueType().hideVersionForVersionColumn(isToSql);
+ String keyDesc = ((MapType) this).getKeyType()
+ .hideVersionForVersionColumn(isToSql, showNestedComment);
+ String valueDesc = ((MapType) this).getValueType()
+ .hideVersionForVersionColumn(isToSql, showNestedComment);
return "map<" + keyDesc + "," + valueDesc + ">";
} else if (isStructType()) {
List<String> fieldDesc = new ArrayList<>();
StructType structType = (StructType) this;
for (int i = 0; i < structType.getFields().size(); i++) {
StructField field = structType.getFields().get(i);
- fieldDesc.add(field.getName() + ":" +
field.getType().hideVersionForVersionColumn(isToSql));
+ StringBuilder desc = new
StringBuilder(field.getName()).append(":")
+
.append(field.getType().hideVersionForVersionColumn(isToSql,
showNestedComment));
+ // Requiredness is schema semantics and must survive
independently of whether
+ // nested documentation is requested for DESCRIBE output.
+ if (!field.getContainsNull()) {
+ desc.append(" not null");
+ }
+ // Nested docs are part of DESCRIBE output only when comments
were explicitly requested.
+ if (showNestedComment && field.isCommentSpecified()) {
+ desc.append(String.format(" comment '%s'",
field.getComment()));
Review Comment:
[P2] Quote nested comments in DESCRIBE type strings
With `show_column_comment_in_describe=true`, a valid nested comment such as
`owner's \path` is rendered as `comment 'owner's \path'`. That is not a valid
round-trippable SQL literal, and backslashes may also be reinterpreted. The
parallel Nereids `StructField.toSql()` uses
`SqlLiteralUtils.quoteStringLiteral` for this syntax. Please use equivalent
SQL-literal quoting here and add coverage for quotes and backslashes.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java:
##########
@@ -206,10 +206,9 @@ public void beginWrite(ConnectorSession session, String
db, String tableName, Ic
this.zone = IcebergTimeUtils.resolveSessionZone(session);
try {
context.executeAuthenticated(() -> {
- // PERF-07: resolve the table through the per-statement
scope so a row-level DML reuses the SAME
- // one object the scan already loaded (a write-only INSERT
loads once here). openTransaction still
- // issues newTransaction()'s refresh, giving the commit a
fresh OCC base off that shared object.
- Table loaded = IcebergStatementScope.sharedTable(session,
db, tableName,
+ // Reuse the write planner's mutable table, never the
frozen read view: newTransaction
+ // refreshes operations to establish a fresh OCC base and
must not mutate bound read metadata.
+ Table loaded =
IcebergStatementScope.sharedWritableTable(session, db, tableName,
Review Comment:
[P1] Preserve an explicitly empty read as the RowDelta base
An empty latest snapshot is `-1`, but `applySnapshot` leaves that handle
unpinned and `buildWriteContext` passes `-1`. With this new independent
writable-table load, a MERGE whose target was bound to empty T0 can load a
concurrent first append S1 here; `applyBeginGuards` then records S1 as
`baseSnapshotId`. The commit calls `validateFromSnapshot(S1)`, so serializable
`validateNoConflictingDataFiles` ignores the matching file added in S1 and the
MERGE can insert a duplicate. Iceberg's `BaseRowDelta` uses an unset starting
snapshot to check all versions, so this edge must preserve "explicitly empty"
separately from "no pin" (or otherwise validate from T0). Please add an
empty-bind / concurrent-first-append MERGE barrier test.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
