Skip to content

refactor: Rework TestClpFilterToKql unit tests related to pushdown, excluding split-filter pushdown. #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: release-0.293-clp-connector
Choose a base branch
from
Open

refactor: Rework TestClpFilterToKql unit tests related to pushdown, excluding split-filter pushdown. #77

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0033c69
Add unit tests
anlowee Oct 6, 2025
d2bec6f
Add unit tests for BETWEEN and fix some format issues
anlowee Oct 6, 2025
a929cd6
Move all unit test cases except metadata (split filter) pushdown from…
anlowee Oct 7, 2025
8cc4449
Remove unrelated changes
anlowee Oct 7, 2025
d89ca3f
Rename
anlowee Oct 7, 2025
58f885e
Merge branch 'release-0.293-clp-connector' into xwei/ensure-ts-nanose…
anlowee Oct 8, 2025
c9d78d1
Address coderabbitai comments
anlowee Oct 8, 2025
27b83ad
Address coderabbitai comments
anlowee Oct 8, 2025
c99be8f
Merge branch 'release-0.293-clp-connector' into xwei/ensure-ts-nanose…
anlowee Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions presto-clp/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.facebook.presto</groupId>
<artifactId>presto-main</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.facebook.presto</groupId>
<artifactId>presto-main-base</artifactId>
Expand Down
48 changes: 40 additions & 8 deletions ...lp/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.facebook.presto.common.function.OperatorType;
import com.facebook.presto.common.type.DecimalType;
import com.facebook.presto.common.type.RowType;
import com.facebook.presto.common.type.TimestampType;
import com.facebook.presto.common.type.Type;
import com.facebook.presto.common.type.VarcharType;
import com.facebook.presto.plugin.clp.ClpColumnHandle;
Expand Down Expand Up @@ -57,12 +58,15 @@
import static com.facebook.presto.common.type.IntegerType.INTEGER;
import static com.facebook.presto.common.type.RealType.REAL;
import static com.facebook.presto.common.type.SmallintType.SMALLINT;
import static com.facebook.presto.common.type.TimestampType.TIMESTAMP;
import static com.facebook.presto.common.type.TimestampType.TIMESTAMP_MICROSECONDS;
import static com.facebook.presto.common.type.TinyintType.TINYINT;
import static com.facebook.presto.plugin.clp.ClpErrorCode.CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION;
import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.AND;
import static java.lang.Integer.parseInt;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
import static java.util.concurrent.TimeUnit.SECONDS;

/**
* A translator to translate Presto {@link RowExpression}s into:
Expand Down Expand Up @@ -256,8 +260,10 @@ private ClpExpression handleBetween(CallExpression node)
}

String variable = variableOpt.get();
String lowerBound = getLiteralString((ConstantExpression) second);
String upperBound = getLiteralString((ConstantExpression) third);
Type lowerBoundType = second.getType();
String lowerBound = tryEnsureNanosecondTimestamp(lowerBoundType, getLiteralString((ConstantExpression) second));
Type upperBoundType = third.getType();
String upperBound = tryEnsureNanosecondTimestamp(upperBoundType, getLiteralString((ConstantExpression) third));
String kql = String.format("%s >= %s AND %s <= %s", variable, lowerBound, variable, upperBound);
String metadataSqlQuery = metadataFilterColumns.contains(variable)
? String.format("\"%s\" >= %s AND \"%s\" <= %s", variable, lowerBound, variable, upperBound)
Expand Down Expand Up @@ -440,6 +446,7 @@ private ClpExpression buildClpExpression(
RowExpression originalNode)
{
String metadataSqlQuery = null;
literalString = tryEnsureNanosecondTimestamp(literalType, literalString);
if (operator.equals(EQUAL)) {
if (literalType instanceof VarcharType) {
return new ClpExpression(format("%s: \"%s\"", variableName, escapeKqlSpecialCharsForStringValue(literalString)));
Expand Down Expand Up @@ -486,7 +493,7 @@ private Optional<ClpExpression> tryInterpretSubstringEquality(
RowExpression possibleSubstring,
RowExpression possibleLiteral)
{
if (!operator.equals(EQUAL)) {
if (!operator.equals(EQUAL) && !(operator.equals(NOT_EQUAL))) {
return Optional.empty();
}

Expand All @@ -501,7 +508,7 @@ private Optional<ClpExpression> tryInterpretSubstringEquality(
}

String targetString = getLiteralString((ConstantExpression) possibleLiteral);
return interpretSubstringEquality(maybeSubstringCall.get(), targetString);
return interpretSubstringEquality(maybeSubstringCall.get(), targetString, operator.equals(EQUAL));
}

/**
Expand Down Expand Up @@ -554,8 +561,12 @@ private Optional<SubstrInfo> parseSubstringCall(CallExpression callExpression)
* @param targetString the literal string being compared to
* @return an Optional containing either a ClpExpression with the equivalent KQL query
*/
private Optional<ClpExpression> interpretSubstringEquality(SubstrInfo info, String targetString)
private Optional<ClpExpression> interpretSubstringEquality(SubstrInfo info, String targetString, boolean isEqual)
{
StringBuilder result = new StringBuilder();
if (!isEqual) {
result.append("NOT ");
}
if (info.lengthExpression != null) {
Optional<Integer> maybeStart = parseIntValue(info.startExpression);
Optional<Integer> maybeLen = parseLengthLiteral(info.lengthExpression, targetString);
Expand All @@ -564,7 +575,6 @@ private Optional<ClpExpression> interpretSubstringEquality(SubstrInfo info, Stri
int start = maybeStart.get();
int len = maybeLen.get();
if (start > 0 && len == targetString.length()) {
StringBuilder result = new StringBuilder();
result.append(info.variableName).append(": \"");
for (int i = 1; i < start; i++) {
result.append("?");
Expand All @@ -579,7 +589,6 @@ private Optional<ClpExpression> interpretSubstringEquality(SubstrInfo info, Stri
if (maybeStart.isPresent()) {
int start = maybeStart.get();
if (start > 0) {
StringBuilder result = new StringBuilder();
result.append(info.variableName).append(": \"");
for (int i = 1; i < start; i++) {
result.append("?");
Expand All @@ -588,7 +597,8 @@ private Optional<ClpExpression> interpretSubstringEquality(SubstrInfo info, Stri
return Optional.of(new ClpExpression(result.toString()));
}
if (start == -targetString.length()) {
return Optional.of(new ClpExpression(format("%s: \"*%s\"", info.variableName, targetString)));
result.append(format("%s: \"*%s\"", info.variableName, targetString));
return Optional.of(new ClpExpression(result.toString()));
}
}
}
Expand Down Expand Up @@ -914,9 +924,31 @@ public static boolean isClpCompatibleNumericType(Type type)
|| type.equals(TINYINT)
|| type.equals(DOUBLE)
|| type.equals(REAL)
|| type.equals(TIMESTAMP)
|| type.equals(TIMESTAMP_MICROSECONDS)
|| type instanceof DecimalType;
}

private static String tryEnsureNanosecondTimestamp(Type type, String literalString)
{
if (type == TIMESTAMP) {
return ensureNanosecondTimestamp(TIMESTAMP, literalString);
}
else if (type == TIMESTAMP_MICROSECONDS) {
return ensureNanosecondTimestamp(TIMESTAMP_MICROSECONDS, literalString);
}
return literalString;
}

private static String ensureNanosecondTimestamp(TimestampType type, String literalString)
{
long literalNumber = Long.parseLong(literalString);
long seconds = type.getEpochSecond(literalNumber);
long nanosecondFraction = type.getNanos(literalNumber);
long nanoseconds = SECONDS.toNanos(seconds) + nanosecondFraction;
return Long.toString(nanoseconds);
}

private static class SubstrInfo
{
String variableName;
Expand Down
Loading
Loading