voonhous commented on code in PR #19835:
URL: https://github.com/apache/hudi/pull/19835#discussion_r3940086655
##########
hudi-common/src/test/java/org/apache/hudi/common/schema/TestHoodieSchemaUtils.java:
##########
@@ -1044,15 +1045,41 @@ public void testGenerateProjectionSchema() {
assertTrue(fieldNames1.contains("timestamp"));
// Field names are matched case-insensitively; HiveHoodieReaderContext
lowercases names before calling this.
- HoodieSchema schema2 =
HoodieSchemaUtils.generateProjectionSchema(originalSchema,
Arrays.asList("_ROW_KEY"));
+ HoodieSchema schema2 =
HoodieSchemaUtils.generateProjectionSchema(originalSchema,
Arrays.asList("PII_COL"));
assertEquals(1, schema2.getFields().size());
- assertEquals("_row_key", schema2.getFields().get(0).name());
+ assertEquals("pii_col", schema2.getFields().get(0).name());
Throwable caughtException = assertThrows(HoodieException.class, () ->
HoodieSchemaUtils.generateProjectionSchema(originalSchema,
Arrays.asList("_row_key", "timestamp", "fake_field")));
assertTrue(caughtException.getMessage().contains("Field fake_field not
found in log schema. Query cannot proceed!"));
}
+ @Test
+ public void testGenerateProjectionSchemaIgnoresDefaultLocale() {
+ // Under tr-TR, String#toLowerCase() maps an upper-case I to dotless-i
(U+0131), so a default-locale lowercase
+ // on one side of the lookup and Locale.ROOT on the other
(HiveHoodieReaderContext) cannot match for any name
+ // that contains an upper-case I. Surefire runs this module single-forked
and sequentially, so toggling the
Review Comment:
You're right, that had it backwards. Reworded to credit the `finally`
restore and dropped the fork claim:
```java
// that contains an upper-case I. Surefire reuses one JVM across the
module's tests, so the finally block below
// is what keeps the toggle from reaching any test that runs after this one.
```
Checked the poms while I was at it: `forkCount`/`reuseForks` are pinned only
in the `functional-tests*` profiles, and this class is untagged so it runs
under `unit-tests`, which pins neither. Surefire's own defaults (`forkCount=1`,
`reuseForks=true`) still give one reused JVM, and there is no
`junit-platform.properties` anywhere in the repo, so nothing runs concurrently
inside it either. The restore should make things safe now as mentioned in the
comment.
Comment is more explicit now and should prevent anyone from removing it
accidentally.
--
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]
