[incubator-pinot] branch master updated: Avoid variable substitution in metadata (#5822)

[ja...@apache.org]

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 <17...@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-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org