gianm commented on a change in pull request #9893:
URL: https://github.com/apache/druid/pull/9893#discussion_r428827872
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -7300,6 +7301,74 @@ public void testRegexpExtract() throws Exception
);
}
+ @Test
+ public void testRegexpExtractFilterViaNotNullCheck() throws Exception
+ {
+ // Cannot vectorize due to extractionFn in dimension spec.
+ cannotVectorize();
+
+ testQuery(
+ "SELECT COUNT(*)\n"
+ + "FROM foo\n"
+ + "WHERE REGEXP_EXTRACT(dim1, '^1') IS NOT NULL OR REGEXP_EXTRACT('Z'
|| dim1, '^Z2') IS NOT NULL",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ expressionVirtualColumn("v0",
"regexp_extract(concat('Z',\"dim1\"),'^Z2')", ValueType.STRING)
+ )
+ .filters(
+ or(
+ not(selector("dim1", null, new
RegexDimExtractionFn("^1", 0, true, null))),
+ not(selector("v0", null, null))
+ )
+ )
+ .aggregators(new CountAggregatorFactory("a0"))
+ .context(TIMESERIES_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{3L}
+ )
+ );
+ }
+
+ @Test
+ public void testRegexpLikeFilter() throws Exception
+ {
+ // Cannot vectorize due to usage of regex filter.
+ cannotVectorize();
+
+ testQuery(
+ "SELECT COUNT(*)\n"
+ + "FROM foo\n"
+ + "WHERE REGEXP_LIKE(dim1, '^1') OR REGEXP_LIKE('Z' || dim1, '^Z2')",
Review comment:
> a multi-value column
There aren't tests for multi-value columns for this specific expression, nor
for other functions that aren't multi-value-aware. (There is a separate system
that handles mapping over non-multi-value-aware functions.) I could see having
a systematic way to test for this (a query generator, maybe) but I don't think
adding one just for this function would make sense.
> a numeric column
There aren't tests for a numeric column. It should be a validation error but
we don't currently have very comprehensive tests for the validator. I could add
one in an ad-hoc way right now, and that would make sense. But I was actually
hoping to add more systematic tests in a future patch, so could do it then too.
> matching against null
I added a test for matching against null to ExpressionsTest.
> The docs say that "The pattern must match starting at the beginning of
expr" but it looks like the regex pattern you are passing in is asking that it
start at the beginning of the string via ^ in the pattern string. Can I use a $
in my regex to ask that it matches the end of the expr?
Wow! After looking into your comment, I realized that I totally screwed up
the docs here. I actually have a test in this patch for what happens when you
skip the `^`, and it _does_ match substrings that are in the middle of the
string, and the test expects that to happen, all in contradiction of what the
docs say. I'll fix the docs. Thanks for calling this to my attention.
And yes, you can use `$` to ask that it match the end. There are tests for
this too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
