You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/04/26 06:20:46 UTC

[GitHub] [druid] LakshSingla commented on a diff in pull request #12431: Validate select columns for insert statement

LakshSingla commented on code in PR #12431:
URL: https://github.com/apache/druid/pull/12431#discussion_r858316198


##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java:
##########
@@ -93,19 +93,20 @@
 import org.apache.druid.utils.Throwables;
 
 import javax.annotation.Nullable;
-
 import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Properties;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 public class DruidPlanner implements Closeable
 {
   private static final EmittingLogger log = new EmittingLogger(DruidPlanner.class);
+  private static final Pattern UNNAMED_COLUMN_PATTERN = Pattern.compile("EXPR\\$\\d+");

Review Comment:
   Would the given pattern match a column name like `abcEXPR$0xyz`? And if so, should we add regex boundaries to the pattern? 



##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java:
##########
@@ -743,6 +743,38 @@ public void testInsertWithInvalidSelectStatement()
         .verify();
   }
 
+  @Test

Review Comment:
   Should we also add a test that tries to ingest a forbidden column name (like EXPR$0)? This would also help in documenting the behavior in the tests, and if we get a workaround to that method, that test would fail and would need to be updated.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org