voonhous commented on code in PR #19495:
URL: https://github.com/apache/hudi/pull/19495#discussion_r3709387333
##########
hudi-trino/src/test/java/io/trino/plugin/hudi/TestPrefilledColumnValues.java:
##########
@@ -81,9 +81,20 @@ public void testHiveNullPartitionValue()
{
// Trino's HivePartitionKey encodes a null partition value as the
literal string "\N"
PrefilledColumnValues values = prefilledValues(new
HivePartitionKey("pk_string", "\\N"));
+ HiveColumnHandle handle = partitionKey("pk_string", VARCHAR,
HiveType.HIVE_STRING);
- Block block = singleValueBlock(values, partitionKey("pk_string",
VARCHAR, HiveType.HIVE_STRING));
+ Block block = singleValueBlock(values, handle);
assertThat(block.isNull(0)).isTrue();
+
+ // Resolved values are memoized per column, and null is a legitimate
resolved value, so a second
Review Comment:
Agreed -- a null-check memo returns null on every call too, so the repeats
don't discriminate. Dropped the claim; the repeats stay, described as what they
actually cover (both read paths keep returning null once the memo is populated).
Went that way rather than the accessor because what it would pin is
perf-only internal state -- a memo that recomputes for hive-null columns is
slower but observably identical -- and this test sits in `io.trino.plugin.hudi`
while the class is in `io.trino.plugin.hudi.util`, so the accessor would have
to be `public`. If you'd rather have the assertion, I'm happy to move the test
into the `util` package (where `TestHudiAvroSerializer` already lives) and add
it package-private.
--
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]
