xiangfu0 commented on code in PR #17998:
URL: https://github.com/apache/pinot/pull/17998#discussion_r3005962904
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/fst/FstIndexTypeTest.java:
##########
@@ -66,35 +68,25 @@ public void oldFieldConfigFstNoType()
+ " \"indexTypes\" : [\"FST\"]\n"
+ " }");
- assertEquals(new FstIndexConfig(null));
+ assertEquals(new FstIndexConfig());
}
@Test
- public void oldFieldConfigFstExplicitLuceneType()
+ public void newNativeTypeIsIgnored()
Review Comment:
These new tests are defending a dead compatibility path. After this PR,
`FstIndexConfig` no longer has a `type` field and `TextIndexConfig` no longer
has an `fst` field, so the only thing these assertions verify is that Jackson
ignores unknown JSON. There is no production logic left behind those fields. I
would rather remove these tests and fail fast at table-config validation if we
still care about legacy `native` selectors, instead of pinning silent-ignore
behavior on dead config surface.
--
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]
