Skip to content

Conversation

@thomasmueller
Copy link
Member

No description provided.

@nit0906
Copy link
Contributor

nit0906 commented May 24, 2023

hi @thomasmueller - I had added a failing IT for this issue early on

    public void testCoalesceWithoutIndex() throws CommitFailedException {
        final String age = "age";
        final String experience = "experience";

        Tree t = root.getTree("/");
        Tree indexDefn = createTestIndexNode("test-index-no-coalesce", t, indexOptions.getIndexType());
        TestUtil.useV2(indexDefn);

        Tree props = TestUtil.newRulePropTree(indexDefn, NT_UNSTRUCTURED);
        TestUtil.enableForOrdered(props, age);
        TestUtil.enableForOrdered(props, experience);

        root.commit();

        List<String> expected = Lists.newArrayList();
        List<String> expected2 = Lists.newArrayList();
        Tree content = root.getTree("/").addChild("content");
        t = content.addChild("test1");
        t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME);
        t.setProperty(age, 1);
        t.setProperty(experience, 1);
        expected.add(t.getPath());
        expected2.add(t.getPath());

        t = content.addChild("test2");
        t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME);
        t.setProperty(age, 2);
        t.setProperty(experience, 6);
        expected.add(t.getPath());
        expected2.add(t.getPath());

        t = content.addChild("test3");
        t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME);
        t.setProperty(experience, 3);
        expected.add(t.getPath());
        expected2.add(t.getPath());

        t = content.addChild("test4");
        t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME);
        t.setProperty(age, 4);
        expected.add(t.getPath());
        expected2.add(t.getPath());

        t = content.addChild("test5");
        t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME);
        t.setProperty(age, 6);
        t.setProperty(experience, 2);

        t = content.addChild("test6");
        t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME);
        t.setProperty(age, 10);

        t = content.addChild("test7");
        t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME);
        t.setProperty(age, 25);
        expected2.add(t.getPath());

        root.commit();

        // asserting the initial state
        for (String s : expected) {
            assertTrue("wrong initial state", root.getTree(s).exists());
        }

        String statement =
                "SELECT * " +
                        "FROM [" + NT_UNSTRUCTURED + "] AS c " +
                        "WHERE " +
                        "( " +
                        "COALESCE(c.[" + age + "], c.[" + experience + "]) < 5)" +
                        " AND ISDESCENDANTNODE(c, '" + content.getPath() + "') " +
                        "ORDER BY age DESC ";
        

        TestUtil.assertEventually(() -> assertQuery(statement, SQL2, expected), 3000 * 5);

        final String statement2 =
                "SELECT * " +
                        "FROM [" + NT_UNSTRUCTURED + "] AS c " +
                        "WHERE " +
                        "( " +
                        "COALESCE(c.[" + age + "], c.[" + experience + "]) < 5" +
                        " OR (COALESCE(c.[" + age + "], c.[" + experience+ "]) = 25))" +
                        " AND ISDESCENDANTNODE(c, '" + content.getPath() + "') " +
                        "ORDER BY age DESC ";


        TestUtil.assertEventually(() -> assertQuery(statement2, SQL2, expected2), 3000 * 5);
    }```

This still seems to fail even after the fix.

Copy link
Contributor

@nit0906 nit0906 left a comment

Choose a reason for hiding this comment

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

LGTM - submitted an IT to the PR as well.

@thomasmueller thomasmueller merged commit b760f76 into trunk May 25, 2023
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.

4 participants