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: [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]
