Conversation
jacktengg
requested review from
924060929,
CalvinKirs,
Gabriel39,
airborne12,
csun5285,
eldenmoon,
englefly,
gavinchou,
liaoxin01,
luwei16,
morrySnow,
mymeiyi,
starocean999 and
yiguolei
as code owners
September 7, 2026 14:36
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Sep 8, 2026
### What problem does this PR solve? Related PR: apache#67627 Problem Summary: UUID dynamic defaults were parsed as literals when partial updates inserted new keys, while ADD COLUMN evaluated one UUID for every existing row. Parquet logical UUID values had no native materialization path, and encoded Variant UUID and nested JSON UUID casts lost valid values. Share batched UUID generation across SQL and all partial-update fill paths, reject volatile UUID defaults for ADD COLUMN, decode Parquet UUID values and dictionaries in canonical byte order, and handle UUID in encoded Variant and JSONB casts. ### Release note Fix UUID defaults for partial-update inserts, Parquet UUID reading, and UUID casts from encoded Variant and nested JSON. ADD COLUMN now rejects UUID dynamic defaults; UUID literal defaults remain supported. ### Check List (For Author) - Test: ASAN BE/FE builds; 106 BE unit tests; 4 FE unit tests; 3 UUID regression suites; generated regression outputs; clang-format v16, FE Checkstyle, and build hygiene checks. clang-tidy was attempted but remains blocked by pre-existing unmatched NOLINTEND and toolchain header errors; details are in review-docs/pr-67627-review-fixes.zh.md. - Regression test / Unit Test - Behavior changed: Yes, correct missing UUID values and reject unstable ADD COLUMN defaults as described above. - Does this need documentation: No, fixes correctness gaps in the UUID feature under review.
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Sep 8, 2026
…ite paths ### What problem does this PR solve? Issue Number: None Related PR: apache#67627 Problem Summary: Partial updates that omit UUID_V4/UUID_V7 columns previously created independent random values on each replica. Persist the shared load ID and derive UUID defaults from the load, primary key and column identity so flush, publish and recovery produce consistent values despite row reordering. Keep native UUID identity in Variant typed encoding, storage-cell assembly and subcolumn materialization, and add the missing FE protobuf mapping for RPC UDF UUID input and output signatures. Legacy metadata without a load ID explicitly fails when UUID dynamic defaults are required; literal defaults remain usable. Variant root scalars retain the existing V1-compatible JSONB storage policy. ### Release note Fix inconsistent UUID defaults across partial-update replicas, preserve native UUID identity in Variant adapters and subcolumns, and allow UUID RPC UDF signatures. ### Check List (For Author) - Test: ASAN BE/FE builds; 58 BE unit tests; 1 FE RPC signature unit test; 5 UUID regression suites including three replicas across 8 write configurations. clang-format v16, FE Checkstyle and build hygiene passed. clang-tidy was run but is blocked by the existing unmatched NOLINTEND in be/src/core/types.h:577 and baseline diagnostics. - Regression test / Unit Test - Behavior changed: Yes, replica-stable UUID defaults and native Variant UUID identity; RPC UDF creation accepts UUID signatures. - Does this need documentation: No
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Sep 8, 2026
### What problem does this PR solve? Issue Number: N/A Related PR: apache#67627 Problem Summary: UUID support added COLUMN_TYPE_UUID to the deprecated PColumnType enum, but no producer or consumer uses this value. IN and MinMax runtime filter serialization writes COLUMN_TYPE_BOOL solely to populate the deprecated required fields. Remove the unused enum member without changing existing field numbers or runtime UUID serialization. ### Release note None ### Check List (For Author) - Test: Manual test: repository reference search, protoc descriptor generation, and git diff --check passed. No runtime tests needed for an unused enum removal. - Behavior changed: No - Does this need documentation: No
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#67627
commit: 864e5448e2236230e662bcddffc10d48acd5fc24
base: 9957987d5bf43fa6a33aa59918435fed65e2912a
reviewed_at: 2026-09-08T18:39+08:00
reviewer: jacktengg
model: gpt-5.6-sol
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers None. Reviewed locally with the |
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Sep 9, 2026
…ization ### What problem does this PR solve? Issue Number: N/A Related PR: apache#67627 Problem Summary: UUID declarations with length or precision parameters were silently accepted. Queries rewritten to multi_distinct_count(UUID) reached an unregistered BE implementation, and UUID MIN/MAX aggregate-storage columns were rejected by FE despite existing BE support. Nested Hive Text UUID values also inherited JSON quoting. Reject unsupported type parameters, register exact UUID distinct aggregation, allow the supported storage aggregates, and separate Hive Text from JSON serialization. Reuse the existing compact-binary writer and preserve its UUID tag and 16-byte payload. Add FE, BE, JNI and regression coverage for type identity, metadata, NULL/Const columns, serialization, distributed operators, actual spill, storage models, compaction, index formats, schema changes, MySQL/Flight clients, Java UDF/UDAF, file round trips and explicit restrictions. Record all 118 checklist items and distinguish local validation from deployment scenarios still requiring external environments. ### Release note UUID now rejects length/precision parameters, supports exact multi-distinct aggregation and MIN/MAX aggregate-storage columns, and emits correct unquoted UUID elements in Hive Text. ### Check List (For Author) - Test: Unit Test / Regression test / Manual test - Standard ASAN BE/FE builds; 92 UUID BE tests and 70 FE/JNI tests passed. - All 15 UUID regression suites passed with generated expected output, including verified physical spill and index/compaction paths. - Independent PyArrow checks for Parquet/ORC OUTFILE and Parquet EXPORT passed; FE plus three BE restart preserved schema and query output. - clang-format 16 and header hygiene passed. clang-tidy remains blocked by the existing unmatched NOLINTEND in core/types.h and reports existing JSONB-test cognitive complexity. - Cloud, mixed-version deployment and external catalog end-to-end acceptance were not performed; details are recorded in docs/uuid-type-completeness.md. - Behavior changed: Yes (UUID parameter validation, distinct/storage aggregates, and Hive Text output) - Does this need documentation: Yes (docs/uuid-type-completeness.md in this commit)
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Sep 9, 2026
…imizations ### What problem does this PR solve? Related PR: apache#67627 Problem Summary: UUID regression suites mixed unrelated features, while result-only checks could pass without partition pruning, bucket pruning, scanner predicates, or indexes actually filtering data. Split the existing suites by feature and add isolated positive/zero profile checks, planner assertions, native UUID Parquet fixtures, invalid-load atomicity, statistics readback, aggregation phases, and cross-batch windows. Map all 118 datatype checklist entries to concrete evidence and record the generic composite-range boundary pruning limitation and remaining external-environment coverage gaps. ### Release note None ### Check List (For Author) - Test: Regression test / Manual test - 54 UUID suites passed on one FE and three ASAN BEs with random order and parallelism 3; zero failures or skips - Generated golden files with run-regression-test.sh and reran without generation flags - Audited preserved golden blocks, independently derived scanner aggregates, and checked Parquet/ORC/EXPORT schema and values with PyArrow 21.0.0 - Source whitespace checks passed; retained the regression generator's blank-line separators in .out files - Behavior changed: No (test coverage and documentation only) - Does this need documentation: Yes (feature coverage index and existing completeness document updated in this commit)
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Sep 9, 2026
Issue Number: None Related PR: apache#67627 Problem Summary: UUID regression cases did not systematically cover special and ordinary values across constant, mixed and column arguments with FE folding, BE folding and folding disabled. Add eight functional matrices and a reusable fixture, including nullable columns, container lambdas, generators, aggregate states, parameter constraints and checks of nondeterministic results. Register UUID in the histogram and map_agg_v2 factories. Refresh MERGE's nested arguments after state rewrites so folding does not leave stale result nullability. Add focused BE/FE unit tests and document coverage and applicable restrictions. Support UUID inputs to HISTOGRAM and UUID keys in MAP_AGG/MAP_AGG_V2. Fix aggregate-state MERGE queries whose argument nullability changes during folding. - Test: Regression test / Unit Test / Manual test - 65 UUID regression suites and 3 existing aggregate-state suites passed. - 39 FE unit tests and 5 BE unit tests passed. - FE and ASAN BE builds, Checkstyle, C++ formatting and build hygiene passed. - Independently audited 93,231 results and folding-mode consistency across 4,397 generated result blocks. - clang-tidy attempted; blocked by the existing unmatched NOLINTEND in be/src/core/types.h:577. - Behavior changed: Yes (UUID aggregate support and correct MERGE result metadata) - Does this need documentation: Yes (docs/uuid-expression-matrix.md in this commit)
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#67627
commit: ea16e060372bbdfa1ebbf484523ba8acfd113348
base: 3763638e6c8e3ebbc1b7079266f0d97213f0278d
reviewed_at: 2026-09-09T15:46+08:00
reviewer: jacktengg
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 16830 ms |
### What problem does this PR solve? Issue Number: None Related PR: apache#67627 Problem Summary: UUID external paths had inconsistent Parquet text and binary carriers, unsafe ClickHouse range pushdown, missing CSV schema support, and missing Iceberg Parquet UUID logical annotations. Preserve Iceberg binary values and statistics, align ordinary Parquet text scans and cache keys, retain ClickHouse UUID ranges locally, and emit interoperable Iceberg UUID schemas. Add source-specific regression matrices and independent Iceberg read-back checks. ### Release note Fix UUID CSV schema support, Parquet scanner consistency, ClickHouse range filtering, and Iceberg Parquet UUID writes. ### Check List (For Author) - Test: Unit Test: 37 FE and 226 BE tests passed; Regression test: 11 suites passed with no skips; 320 generated result blocks independently checked. FE and BE ASAN builds, Checkstyle, clang-format 16 and build hygiene passed. clang-tidy omitted as requested. - Behavior changed: Yes; UUID external values, range filtering and Iceberg file annotations follow the documented carrier semantics. - Does this need documentation: Yes; external_table-current-status-0910.md saved in the requested local documentation directory.
Issue Number: None Related PR: apache#67627 Problem Summary: Native UUID lacked the requested parsing fallback, and UUIDv7 time conversion functions. Add FE signatures and deterministic folding together with BE constant, nullable and column execution. Generate UUIDv7 values per row from the supplied session-local datetime, including counter overflow continuity, and decode timestamps with an optional constant timezone. Extend UUID regression matrices with nested casts, subqueries, grouping sets, NULL ordering, pattern predicates and unary expression contexts. Keep the pre-existing empty-REGEXP STRING discrepancy outside this change and test UUID/text parity for it. Add dateTimeToUUIDv7, UUIDv7ToDateTime, toUUIDOrZero, toUUIDOrNull, and toUUIDOrDefault for UUID, with underscore aliases. Existing UUID_TO_INT/INT_TO_UUID behavior is unchanged. - Test: Unit Test / Regression test / Manual test - FE: 42 tests passed via run-fe-ut.sh. - BE: 8 UUID tests passed via run-be-ut.sh, including overflow continuation. - Regression: 19 new/existing UUID suites passed; the three new suites were verified again after the final BE changes, then the expression and conversion suites reverified after the matrix helper change. All 1195 generated result blocks were checked against an independent oracle (93 check empty-REGEXP text parity after query execution). - FE and ASAN BE builds, FE Checkstyle, scoped clang-format and build hygiene passed. - clang-tidy was not run as requested.
Issue Number: None Related PR: apache#67627 Problem Summary: The new native UUID type still formatted values as text at Arrow and Python boundaries. Represent it as the standard arrow.uuid extension over canonical big-endian fixed_size_binary(16), including nested containers, Flight schema exchange, and Python scalar, vector, aggregate and table functions. Build extension-aware empty and control batches instead of unsupported extension builders. Write UUID Parquet annotations and ORC binary values with logical type attributes, including FE-provided OUTFILE schemas and both ORC readers. Preserve ordinary STRING, public textual interfaces, and legacy external STRING mappings. This reduces the uncompressed UUID value payload from 36 to 16 bytes without claiming a measured end-to-end performance improvement. Native UUID uses binary Arrow/Flight buffers and uuid.UUID Python UDF objects. Parquet and ORC exports preserve 16-byte UUID payloads and UUID type metadata. Existing SQL STRING values and MySQL/JSON/CSV text behavior are unchanged. - Test: ASAN BE+FE build; 135 BE unit tests; 13 FE unit tests; 14 regression suites; independent Parquet/ORC schema and byte inspection; clang-format 16 and build hygiene checks. clang-tidy remains blocked by pre-existing diagnostics including an unmatched NOLINTEND; locally suppress GTest assertion-expansion complexity. - Behavior changed: Yes, native UUID binary interchange and Python object mapping. - Does this need documentation: Yes, updated the requested local document /mnt/disk2/tengjianping/docs/uuid/uuid-binary-proto.md.
Issue Number: None Related PR: apache#67627 Problem Summary: UUID SELECT INTO OUTFILE tests fell back to S3 when custom local path settings were absent. Make CSV, Parquet, ORC and nested-map round trips local-only, following the existing OUTFILE test configuration gate. Discover the BE secure file directory, export into unique local temporary directories, read through LOCAL TVF, and clean up files while retaining tables for debugging. Preserve schema inference and legacy ORC reader checks without requiring UUID-specific path settings. None - Test: Regression test - run-regression-test.sh --run --conf output/uuid-binary-cluster/regression-conf.groovy -d datatype_p0/uuid -s test_uuid_csv_roundtrip,test_uuid_parquet_roundtrip,test_uuid_orc_roundtrip,test_uuid_file_maps: 4 passed, 0 failed, 0 skipped - git diff --check: passed - Behavior changed: No (test-only change) - Does this need documentation: No
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Reading an ORC UUID column with enable_mapping_varbinary=true
fails with the V2 scanner: schema inference produces VARBINARY, but the ORC
reader ignores the option and exposes UUID, causing an unsupported UUID to
VARBINARY cast. Pass the existing option through both the data-file and Iceberg
delete-file reader factories and prioritize it over the UUID annotation.
Implement native VARBINARY ORC decoding, including direct and encoded string
batches, so the mapped column can actually be materialized without text
conversion. Preserve original bytes, NULLs, selected rows and nested types.
### Release note
Fix ORC reads with enable_mapping_varbinary=true in the V2 file scanner,
including UUID-annotated fields and Iceberg delete files.
### Check List (For Author)
- Test: Regression test / Unit Test / Manual test
- Reproduce the unsupported UUID to VARBINARY cast before the fix
- ASAN BE build via build.sh --be -j16
- 188 BE unit tests pass for ORC readers, VARBINARY SerDe and Iceberg UUID/binary paths
- 5 local UUID OUTFILE/LOCAL TVF regression suites pass; no remote storage
- Generate the new golden output with the regression runner and verify all four scanner/mapping combinations
- clang-format v16 and build hygiene checks pass
- clang-tidy attempted; blocked by existing unmatched NOLINTEND and missing stddef.h diagnostics
- Behavior changed: Yes (honor the existing ORC binary mapping option in V2; default mapping is unchanged)
- Does this need documentation: No (restore the existing option's semantics)
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: The UUID array and scalar matrix golden files exceed the
repository's 1 MiB added-file limit at 1,748,503 and 1,564,821 bytes. Move array
element/capture/construction cases and scalar ternary cases into independent
suites with their own tables and golden files. Reuse the existing matrix runner
to preserve every input, constant/column mask, nullability variant and FE/BE/
runtime folding mode. Keep complete row-level oracles instead of hashes or
sampling. All four resulting fixtures are below 1 MiB.
### Release note
None
### Check List (For Author)
- Test: Regression test / Manual test
- Generate all four golden files with run-regression-test.sh -forceGenOut
- Re-run all four suites normally: 4 passed, 0 failed, 0 skipped
- Compare all 2,442 original tagged result blocks and ordered row contents against the split fixtures: identical
- Check fixture sizes: 839,412 / 909,186 / 893,071 / 671,845 bytes, all below 1,048,576
- Check patch whitespace; allow the regression generator's final blank lines
- Behavior changed: No (test organization only; query coverage and expected values are unchanged)
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Rebasing UUID support onto the JNI plugin isolation refactor
moves the production data-plane classes into jni-spi, but leaves the new UUID
tests under the removed java-common module. Those tests would no longer run,
and their old package imports and JUnit 4 dependency are incompatible with the
new modules. Move the vector and byte-order tests to jni-spi using JUnit 5,
and keep the Java UDF candidate-type assertion in the java-udf module so the
shared SPI retains its zero-dependency boundary.
The UUID type and accessors also extend the newly frozen JNI plugin surface.
Refresh its baseline from JniPluginSurfaceTest output, bump the shared API
major to 4.0 as required by the new contract, and document the UUID type string
and fixed-width off-heap encoding.
### Release note
UUID-enabled JNI plugins declare API version 4.0 and require a matching shared
JNI SPI. UUID value semantics and byte encoding are unchanged.
### Check List (For Author)
- Test: Unit Test / Manual test
- run-fe-ut.sh: 58 targeted tests pass across UUID literals/types, vector and byte conversion, SPI surface/version, plugin runtime and Java UDF integration
- Checkstyle passes for jni-spi and java-udf
- BE build hygiene passes
- Verify origin/master is an ancestor, no conflict markers remain, and the original untracked-file inventory is unchanged
- Full BE build and regression tests not run
- Behavior changed: Yes (JNI plugin API major becomes 4.0; UUID semantics are unchanged)
- Does this need documentation: Yes (fe/be-java-extensions/jni-spi/PROTOCOL.md)
### What problem does this PR solve?
Issue Number: N/A
Problem Summary: Reusing a server-prepared short-circuit query replaces the
cached UUID predicate with a StringLiteral from the MySQL parameter. Tablet
pruning then hashes UTF-8 text instead of the UUID's binary representation,
so existing rows can be missed. Retyping the key during RPC serialization is
too late because the tablet has already been selected. Reuse the existing
key-literal conversion before updating scan conjuncts, keeping pruning and
lookup keys consistent without disabling the short-circuit path.
### Release note
Fix missing rows when reusing prepared point queries on UUID keys.
### Check List (For Author)
- Test: Regression test / Unit Test / Manual test
- 22 focused FE unit tests passed, including four new rebinding tests.
- Five regression suites passed; 40 generated UUID prepared results verified.
- The supplied reproducer passed all four cases on an isolated cluster.
- FE build and Checkstyle passed; golden results generated by the regression runner.
- Behavior changed: Yes, prepared UUID point queries route to the correct tablet.
- Does this need documentation: No, restores existing query semantics.
### What problem does this PR solve?
Issue Number: N/A
Problem Summary: The newly introduced UUID type transports expression and
partition-boundary literals as text, requiring BE parsing and formatting.
Replace the UUID Thrift string with required high and low i64 fields and
update all FE and BE producers and consumers. Preserve all 128 bits by
joining unsigned halves, including values whose signed Thrift halves are
negative. Keep SQL text conversion and persisted UUID representations
unchanged. Directly replace the branch-local protocol without a legacy
string fallback.
### Release note
UUID literal transport now uses two Thrift integers. FE and BE must be
updated together; the previous UUID string-literal protocol is incompatible.
SQL UUID semantics and stored data formats are unchanged.
### Check List (For Author)
- Test: Regression test / Unit Test / Manual test
- 11 focused FE unit tests and 23 ASAN BE unit tests passed.
- Ten regression suites passed, including UUID folding, partitioning,
runtime filtering and both UUID and existing-type prepared point queries.
- All four supplied prepared-query reproduction cases passed.
- New golden results were generated by the regression runner and verified.
- FE and ASAN BE builds, Checkstyle, clang-format 16, header hygiene and
git diff whitespace checks passed.
- Clang-tidy was run; its gate remains blocked by the pre-existing unmatched
NOLINTEND in be/src/core/types.h. New test complexity and failed-fixture
initialization diagnostics were fixed; final changed-line diagnostics are zero.
- Behavior changed: Yes, UUID literal wire encoding changes; SQL behavior does not.
- Does this need documentation: No, internal branch-local wire representation only.
### What problem does this PR solve?
Issue Number: N/A
Problem Summary: The BE implementation of is_uuid accepted any 36-character
string whose characters are hex digits, dashes at the separator positions or a
mix of both, because it only rejected non-hex characters that were not located
at the four separator positions. A 36-character value consisting of hex digits
only, such as 36 zeros, therefore returned true even though it is neither the
32-character dash-less form nor the canonical 8-4-4-4-12 form. The FE constant
folding path already rejected that value, so is_uuid('000...0') and is_uuid(v)
for the same text disagreed. Require the four separators to be dashes at the
fixed positions and hex digits everywhere else, which matches the FE
implementation and the documented formats.
### Release note
is_uuid now returns false for 36-character UUID strings that miss the four
fixed dashes, consistent with constant folding and the documented input
formats. Valid 32-character, 36-character and braced 38-character forms are
unchanged.
### Check List (For Author)
- Test: Regression test / Manual test
- New regression suite test_uuid_is_uuid passed against the worktree cluster;
it covers the real column path and the literal path with constant folding
enabled and disabled (debug_skip_fold_constant), and its golden .out file
was generated by the regression runner.
- Cases for the affected inputs were added to the existing BE unit test
function_uuid_test.function_is_uuid_test. They were not executed here
because building the BE unit test binary was out of scope for this change.
- Manual SQL verification on the worktree cluster: the reported 36-character
dash-less value returns false on both the column and literal paths, while
the 32-character dash-less and canonical 36-character forms still return true.
- Behavior changed: Yes, 36-character UUID strings without the fixed dashes now
report false instead of true.
- Does this need documentation: No
Issue Number: N/A
Related PR: N/A
Problem Summary: Creating a UUID RANGE table with dynamic partition properties
previously succeeded with no partitions. The scheduler caught the unsupported
column type error, recorded ERROR, and left the table unable to accept writes.
Validate the partition column with the scheduler's existing getPartitionFormat
check at the shared dynamic property analysis entry point, covering CREATE and
ALTER before saving metadata. Limit the check to dynamic properties so ordinary
UUID RANGE tables remain valid, and skip the new check during log replay.
Add utility tests and regression coverage for rejected UUID DDL, unchanged
static UUID tables, replay compatibility, and working DATE dynamic partitions.
Reject dynamic partition CREATE and ALTER statements with unsupported partition
column types synchronously instead of leaving tables in scheduler ERROR state.
- Test: Regression test / Unit Test / Manual test
- 55 FE unit tests in DynamicPartitionUtilTest and DynamicPartitionTableTest
- Four UUID and three existing dynamic partition regression suites passed
- Generated the new golden file with the regression runner and compared it
- Reproduced the pre-fix ERROR/empty-partition behavior on an isolated cluster
- Standard FE build with Checkstyle and source whitespace checks passed
- Behavior changed: Yes, unsupported dynamic partition types fail during DDL
- Does this need documentation: No, enforce the existing supported type contract
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Defer UUID partition keys until a concrete use case requires
support. Reject UUID columns in RANGE and LIST partitions, including automatic
LIST partitions, through the existing partition type validation. Remove the
UUID partition successor and BE partition boundary handling. Replace the
positive UUID partition suites with rejection coverage and a date-partitioned
table that stores and distributes UUID values. Preserve UUID hash distribution
and runtime-filter expression result types.
### Release note
UUID columns can no longer be used as RANGE or LIST partition keys. UUID hash
distribution remains supported.
### Check List (For Author)
- Test: Unit Test / Regression test
- Build: ./build.sh --be --fe -j 16 (ASAN; FE Checkstyle passed)
- FE unit tests: 55 passed across partition metadata, UUID literals, and point queries
- BE unit tests: 17 passed across tablet sink partitioning and runtime partition pruning
- Regression: test_uuid_partition, test_uuid_bucket_pruning,
test_uuid_runtime_bucket_pruning, test_uuid_comparison, and
test_uuid_type_contract passed; new output generated by the regression runner
- clang-format v16 and build hygiene checks passed
- clang-tidy attempted with the toolchain resource path; blocked by the
pre-existing unmatched NOLINTEND in be/src/core/types.h:577. No diagnostics
were reported on added or modified lines.
- Behavior changed: Yes, reject UUID partition keys
- Does this need documentation: No
Issue Number: None Related PR: None Problem Summary: UUID casts and text serializers previously allocated a temporary 36-character string for each value and copied it into the destination column. Formatting also walked every output position, tested for separators, and shifted a 128-bit integer for each hexadecimal digit. Add a caller-buffer formatter that uses the existing hexadecimal byte-pair lookup and fixed 64-bit groups, and let CastToString write directly into BufferWritable. The string-returning API shares the same formatter. Numeric and network-byte conversions are already independent of host endianness; document that contract and cover it with fixed vectors, unaligned buffers, boundary values, and 4096 comparisons against Boost. Add SQL coverage for stored nullable UUIDs, nested serializers and 4096 rows of byte-pair formatting. Canonical text and serialized representations are unchanged. No throughput claim is made: the benchmark target requires RELEASE, while this change was built and tested with ASAN. Reduce temporary allocations and per-digit work when formatting UUID values. - Test: ASAN BE build via build.sh --be -j 16; 35 BE unit tests via run-be-ut.sh; five regression suites via run-regression-test.sh (test_uuid_format, test_uuid_cast, test_uuid_json, test_uuid_nested_types, test_uuid_type_semantics). New expected output was generated and verified by the regression runner. clang-format 16 and build hygiene checks passed. clang-tidy was run and has no diagnostics on changed lines, but its full check remains blocked by the existing unmatched NOLINTEND in core/types.h:577; it also reports an existing UUID generation comparison outside the changed lines. - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: When projection pruning leaves no required scan slots,
PhysicalPlanTranslator retains the smallest available slot. UUID was missing
from the fixed-width priority group, so string and complex slots were preferred
over a 16-byte UUID slot. Include Type.isUuid() in that group and preserve the
existing size comparison within it. Add comparator coverage against string and
complex types, fixed-width size ordering, and a COUNT(*) scan-slot plan case.
### Release note
Prefer UUID over string and complex columns when choosing a fallback scan slot
for COUNT(*) and other queries with no required scan columns.
### Check List (For Author)
- Test: Unit Test
- run-fe-ut.sh --run org.apache.doris.nereids.glue.translator.CountStarSmallestSlotTest: all 19 tests passed; the two new UUID tests failed before the fix.
- build.sh --fe -j8 with FE_MAVEN_THREADS=8 and DISABLE_BE_JAVA_EXTENSIONS=ON: passed, including FE Checkstyle.
- git diff --check: passed.
- Behavior changed: Yes (fallback scan-slot selection now classifies UUID with fixed-width types; query results are unchanged).
- Does this need documentation: No
Issue Number: N/A Related PR: apache#68246, apache#67805 Problem Summary: The UUID aggregate matrix still expected an empty array from TOPN_ARRAY(uuid, 100, 0), reflecting the old behavior that discarded every candidate while serializing a zero-capacity partial state. Current BE code treats non-positive expansion rates as unlimited candidate retention, so the test fails on a current BE while passing against an older local binary. Regenerate the expected output using the regression runner, name the third argument as an expansion rate, sort the returned arrays, and cover a negative rate in the existing phase/folding/input matrix. Replace the obsolete COLLECT_SET column-limit matrix with an expected-error case, matching the FE requirement that the limit argument is constant. None - Test: Regression test and Manual test - Reproduced unary_p1_fe_c_1 against the unchanged expected output after rebuilding the current BE; the old local BE passed the old test. - run-regression-test.sh --run -d datatype_p0/uuid -s test_uuid_aggregate_matrix -forceGenOut: passed. - run-regression-test.sh --run -d datatype_p0/uuid,nereids_function_p0/agg_function/topn -s test_uuid_aggregate_matrix,topn_unlimited: 2 suites passed. - Verified 66 unary matrix queries against default TOPN results and confirmed all 584 other retained output blocks are unchanged. - build.sh --be -j48 (ASAN) and git diff --check: passed. - Behavior changed: No (test-only alignment with existing aggregate parameter semantics). - Does this need documentation: No
Contributor
|
run buildall |
Contributor
Author
|
/review |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Static review of UUID support on head 3044575863e038595742b9f736868bb9ee07f3e2: requesting changes for one correctness issue in the Arrow Flight schema path.
Checkpoint conclusions:
- Goal, focusedness, and tests: the PR broadly implements native UUID across FE, BE, storage, formats, functions, and UDF/JNI paths, with substantial unit/regression coverage. However, the new Flight schema assertion exposes an omitted central mapper case, so the Flight portion does not yet accomplish its goal and its own test cannot pass.
- Parallel paths and conditions: V1/V2 Parquet and ORC, Arrow/Iceberg carriers, runtime filters/indexes, point queries, aggregates, casts, JNI/JDBC/Python, and fixed/flexible partial updates were traced. No additional distinct correctness issue was substantiated; existing review threads remain duplicate fences.
- Concurrency, lifecycle, and performance: UUIDv7 atomic/counter state, the scoped datetime-to-v7 mutex, Arrow extension lifecycle, and partial-update load identity/replay were reviewed. No additional concurrency, lifetime, or material performance defect was found.
- Configuration and compatibility: no new dynamic configuration contract is introduced. ORC varbinary precedence and the mixed-version BE fence are already covered by existing threads; JNI correctly advances its plugin API major version.
- Persistence, transactions, and atomicity: the partial-update load seed is persisted and restored, and deterministic defaults use logical keys across replica, retry, row-remap, and rollup paths. No additional transaction, EditLog, replay, or crash-consistency issue was found.
- Observability and FE/BE variables: no new metric, logging, or FE-to-BE variable gap was identified for the added paths.
- User focus: no additional focus was supplied.
Validation was static only, as required by the review prompt; no build or test command was run. Live CI state is not treated as independent execution evidence.
| desc("key", TPrimitiveType.UUID), desc("value", TPrimitiveType.UUID))), | ||
| buildField(desc("record", TPrimitiveType.STRUCT, desc("u", TPrimitiveType.UUID))), | ||
| buildField(desc("text", TPrimitiveType.STRING)))); | ||
| Schema schema = MessageSerializer.deserializeSchema( |
Contributor
There was a problem hiding this comment.
[P1] Add UUID to the central Flight Arrow type mapping. buildField(...) reaches DorisArrowTypeMapping.toArrowType, whose switch currently has no PrimitiveType.UUID case and therefore returns ArrowType.Null. That means GetTables and nested serialized schemas advertise Null while BE emits arrow.uuid, and this assertion (plus the existing exhaustive everyPrimitiveTypeHasARow test) cannot pass. Please return UuidType.INSTANCE from the mapper and add UUID to its mapping table/tests.
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 27709 ms |
Contributor
TPC-DS: Total hot run time: 152308 ms |
Contributor
ClickBench: Total hot run time: 23.97 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Document:
apache/doris-website#4131
apache/doris-website#4166
Doris does not have a native UUID SQL and storage type, so UUID values must be stored as strings and cannot use a fixed-width representation or UUID-aware generation and version functions. Add a native 128-bit UUID type across FE, BE, IDL, storage, serialization, expressions, runtime filters, external and JNI bridges, and schema metadata. Accept canonical and compact hexadecimal input, normalize output to lowercase canonical form, preserve unsigned canonical ordering, and add native UUID v4 and v7 generation and version inspection while keeping the existing string-returning uuid() behavior compatible.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)