Skip to content

IGNITE-28526 Fix flaky RexSimplificationPlannerTest.testExtractCommonDisjunctionPart#13011

Open
zstan wants to merge 1 commit intoapache:masterfrom
zstan:ignite-28526
Open

IGNITE-28526 Fix flaky RexSimplificationPlannerTest.testExtractCommonDisjunctionPart#13011
zstan wants to merge 1 commit intoapache:masterfrom
zstan:ignite-28526

Conversation

@zstan
Copy link
Copy Markdown
Contributor

@zstan zstan commented Apr 13, 2026

plan from failed logs:

IgniteIndexScan(table=[[PUBLIC, T1]], index=[IDX1], filters=[AND(=($t1, 0), =($t0, 0), CASE(IS NULL($t2), null:BOOLEAN, OR(=(CAST($t2):INTEGER NOT NULL, 0), =(CAST($t2):INTEGER NOT NULL, 1), 
=(CAST($t2):INTEGER NOT NULL, 2))))], requiredColumns=[{0, 1, 2}], searchBounds=[[ExactBounds [bound=0], null, null]], inlineScan=[false], collation=[[0 ASC-nulls-first]]): rowcount = 4341.09375, cumulative cost = IgniteCost [rowCount=75001.0, cpu=300040.3670901322, memory=1.0, io=1.0, network=1.0], id = 1244437

plan from common master branch run:

IgniteIndexScan(table=[[PUBLIC, T1]], index=[IDX1], filters=[AND(=($t0, 0), =($t1, 0), CASE(IS NULL($t2), null:BOOLEAN, OR(=(CAST($t2):INTEGER NOT NULL, 0), =(CAST($t2):INTEGER NOT NULL, 1), 
=(CAST($t2):INTEGER NOT NULL, 2))))], requiredColumns=[{0, 1, 2}], searchBounds=[[ExactBounds [bound=0], null, null]], inlineScan=[false], collation=[[0 ASC-nulls-first]]): rowcount = 4341.09375, cumulative cost = IgniteCost [rowCount=75001.0, cpu=300040.3670901322, memory=1.0, io=1.0, network=1.0], id = 429 

here we can see that conditions are different: filters=[AND(=($t1, 0), =($t0, 0) vs filters=[AND(=($t0, 0), =($t1, 0)

@sonarqubecloud
Copy link
Copy Markdown

boolean firstCall = true;

@Override public RexNode visitCall(RexCall call) {
if (firstCall) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we check only first call, why do we need RexShuttle at all?
It can be implemented much simplier by checking only root operator and operands.
But It still will be too specialized and seems that can be used only by one test.
Instead I propose to implement something like condition normalizator (I think it will be useful in future for other tests), it can be implemented for example this way:

    private static RexNode normalizeCondition(RexBuilder rexBuilder, RexNode condition) {
        return new RexShuttle() {
            @Override public RexNode visitCall(RexCall call) {
                Pair<SqlOperator, List<RexNode>> normalized = RexNormalize.normalize(call.getOperator(), call.getOperands());

                return rexBuilder.makeCall(normalized.getKey(), normalized.getValue());
            }
        }.apply(condition);
    }

And to check condition we can do something like:

    protected <T extends RelNode> Predicate<ProjectableFilterableTableScan> satisfyCondition(String condition) {
        return node -> {
            String normCond = normalizeCondition(node.getCluster().getRexBuilder(), node.condition()).toString();

            if (!condition.equals(normCond)) {
                lastErrorMsg = "Unexpected condition [expected=" + condition + ", actual=" + normCond + ']';

                return false;
            }

            return true;
        };
    }

In this case we can check normalized condition like:

.and(satisfyCondition("AND(=($t0, 0), =($t1, 0), SEARCH($t2, Sarg[0, 1, 2]))")));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but ... it doesn`t work as expected (

            RexBuilder builder = node.getCluster().getRexBuilder();
            RexLiteral l0 = builder.makeExactLiteral(new BigDecimal(0));
            RexLocalRef lr0 = new RexLocalRef(0, node.getCluster().getTypeFactory().createJavaType(long.class));

            RexLiteral l1 = builder.makeExactLiteral(new BigDecimal(0));
            RexLocalRef lr1 = new RexLocalRef(1, node.getCluster().getTypeFactory().createJavaType(long.class));

            final RangeSet<Integer> setNone = ImmutableRangeSet.of();
            final RangeSet<Integer> setAll = setNone.complement();
            final Sarg<Integer> sarg =
                Sarg.of(RexUnknownAs.FALSE, setAll);
            RexNode intLiteral =
                builder.makeLiteral(1, node.getCluster().getTypeFactory().createSqlType(SqlTypeName.INTEGER));
            RexNode rexNode =
                builder.makeCall(SqlStdOperatorTable.SEARCH, intLiteral,
                    builder.makeSearchArgumentLiteral(sarg, intLiteral.getType()));
            RexLocalRef lr2 = new RexLocalRef(2, node.getCluster().getTypeFactory().createJavaType(long.class));

            RexNode r0 = builder.makeCall(SqlStdOperatorTable.EQUALS, lr0, l0);
            RexNode r1 = builder.makeCall(SqlStdOperatorTable.EQUALS, lr1, l1);
            RexNode r3 = builder.makeCall(SqlStdOperatorTable.EQUALS, lr2, rexNode);

            RexNode res = builder.makeCall(SqlStdOperatorTable.AND, r1, r0, r3);
            String norm = normalizeExpression(node.getCluster().getRexBuilder(), res).toString();

            System.err.println("node >>: " + res);
            System.err.println("norm >>: " + norm);

returns:

node >>: AND(=($t1, 0), =($t0, 0), =($t2, SEARCH(1, Sarg[IS NOT NULL])))
norm >>: AND(=($t1, 0), =($t0, 0), =($t2, SEARCH(1, Sarg[IS NOT NULL])))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants