This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push: new ae2bd2f Avoid variable substitution in metadata (#5822) ae2bd2f is described below commit ae2bd2f82f0cd8c19532a0c72944acb7f3d19b61 Author: Xiaotian (Jackie) Jiang <17555551+jackie-ji...@users.noreply.github.com> AuthorDate: Thu Aug 6 12:16:21 2020 -0700 Avoid variable substitution in metadata (#5822) In the segment metadata and column metadata, we always store the actual value in the property file and never use variable substitution (`${anotherKey}`). Using variable substitution can cause problem for special values such as `$${` where the first `$` is identified as escape character and removed. Replace `getString()` with `getProperty()` for column min/max value to avoid variable substitution. --- .../pinot/core/segment/index/metadata/ColumnMetadata.java | 9 ++++++--- .../creator/impl/SegmentColumnarIndexCreatorTest.java | 12 ++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/metadata/ColumnMetadata.java b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/metadata/ColumnMetadata.java index 43ea2a2..d8bf349 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/metadata/ColumnMetadata.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/metadata/ColumnMetadata.java @@ -122,9 +122,12 @@ public class ColumnMetadata { builder.setDateTimeGranularity(dateTimeGranularity); } - // Set min/max value if available. - String minString = config.getString(getKeyFor(column, MIN_VALUE), null); - String maxString = config.getString(getKeyFor(column, MAX_VALUE), null); + // Set min/max value if available + // NOTE: Use getProperty() instead of getString() to avoid variable substitution ('${anotherKey}'), which can cause + // problem for special values such as '$${' where the first '$' is identified as escape character. + // TODO: Use getProperty() for other properties as well to avoid the overhead of variable substitution + String minString = (String) config.getProperty(getKeyFor(column, MIN_VALUE)); + String maxString = (String) config.getProperty(getKeyFor(column, MAX_VALUE)); if (minString != null && maxString != null) { switch (dataType) { case INT: diff --git a/pinot-core/src/test/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreatorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreatorTest.java index eb6da38..b29886f 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreatorTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreatorTest.java @@ -69,6 +69,14 @@ public class SegmentColumnarIndexCreatorTest { assertTrue(SegmentColumnarIndexCreator.isValidPropertyValue("")); testPropertyValueWithSpecialCharacters(""); + // Variable substitution (should be disabled) + assertTrue(SegmentColumnarIndexCreator.isValidPropertyValue("$${testKey}")); + testPropertyValueWithSpecialCharacters("$${testKey}"); + + // Escape character for variable substitution + assertTrue(SegmentColumnarIndexCreator.isValidPropertyValue("$${")); + testPropertyValueWithSpecialCharacters("$${"); + for (int i = 0; i < NUM_ROUNDS; i++) { testPropertyValueWithSpecialCharacters(RandomStringUtils.randomAscii(5)); testPropertyValueWithSpecialCharacters(RandomStringUtils.random(5)); @@ -80,11 +88,11 @@ public class SegmentColumnarIndexCreatorTest { if (SegmentColumnarIndexCreator.isValidPropertyValue(value)) { PropertiesConfiguration configuration = new PropertiesConfiguration(CONFIG_FILE); configuration.setProperty(PROPERTY_KEY, value); - assertEquals(configuration.getString(PROPERTY_KEY), value); + assertEquals(configuration.getProperty(PROPERTY_KEY), value); configuration.save(); configuration = new PropertiesConfiguration(CONFIG_FILE); - assertEquals(configuration.getString(PROPERTY_KEY), value); + assertEquals(configuration.getProperty(PROPERTY_KEY), value); } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org