Skip to content
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.

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

Possible qual postponing past ANTI-JOIN/LASJ_NOTIN-JOIN #13722

Merged
merged 1 commit into from Jan 19, 2023

Conversation

dh-cloud
Copy link
Contributor

When optimizer is off.

For qual (NOT(EXISTS_SUBLINK) and EXPR_SUBLINK)
or qual (NOT(ANY_SUBLINK) AND EXPR_SUBLINK)

In pull_up_sublinks_qual(), after NOT(EXISTS_SUBLINK) is converted to
Anti-JOIN (or NOT(ANY_SUBLINK) => LASJ_NOTIN-JOIN), if EXPR_SUBLINK is
also successfully converted to INNER-JOIN (see convert_EXPR_to_join)
, we can encounter the scenario where qual is postponed past the anti
or lasj_notin join. It will trigger assertion failure:

Assert(j->jointype == JOIN_INNER || j->jointype == JOIN_SEMI);

FATAL: Unexpected internal error (assert.c:48)
DETAIL: FailedAssertion("!(j->jointype == JOIN_INNER || j->jointype == JOIN_SEMI)", File: "initsplan.c", Line: 1018)
server closed the connection unexpectedly
...

We can easily reproduce this issue in master and 6X branch:

CREATE TABLE A(id int, tt timestamp)
  DISTRIBUTED BY (id);

SET optimizer TO off;

-- j->jointype == JOIN_LASJ_NOTIN
SELECT
  t1.*
FROM
  A t1
WHERE
  t1.id NOT IN (SELECT id FROM A WHERE id != 4)
  AND
  t1.tt = (SELECT MIN(tt) FROM A WHERE id = t1.id);

-- j->jointype == JOIN_ANTI
SELECT
  t1.*
FROM
  A t1
WHERE
  NOT EXISTS (SELECT id FROM A WHERE id != 4 and id = t1.id)
  AND t1.tt = (SELECT MIN(tt) FROM A WHERE id = t1.id);

Add these two jointype into the Assert(). I made several simple
tests, results are ok. However, same as the code comment said, i
am also unsure if postponing quals past anti-join/lasj_notin-join
is always semantically correct.

Here are some reminders before you submit the pull request

  • Add tests for the change
  • Document changes
  • Communicate in the mailing list if needed
  • Pass make installcheck
  • Review a PR in return to support the community

@kainwen kainwen added the community PR from outside VMware label Jun 24, 2022
@fanfuxiaoran fanfuxiaoran self-assigned this Sep 28, 2022
When optimizer is off.

For qual (NOT(EXISTS_SUBLINK) and EXPR_SUBLINK)
  or qual (NOT(ANY_SUBLINK) AND EXPR_SUBLINK)

In pull_up_sublinks_qual(), after NOT(EXISTS_SUBLINK) is converted to
Anti-JOIN (or NOT(ANY_SUBLINK) => LASJ_NOTIN-JOIN), if EXPR_SUBLINK is
also successfully converted to INNER-JOIN (see convert_EXPR_to_join)
, we can encounter the scenario where qual is postponed past the anti
or lasj_notin join. It will trigger assertion failure:

  Assert(j->jointype == JOIN_INNER || j->jointype == JOIN_SEMI);

> FATAL:  Unexpected internal error (assert.c:48)
> DETAIL:  FailedAssertion("!(j->jointype == JOIN_INNER || j->jointype == JOIN_SEMI)", File: "initsplan.c", Line: 1018)
> server closed the connection unexpectedly
> ...

We can easily reproduce this issue in master and 6X branch:

```
CREATE TABLE A(id int, tt timestamp)
  DISTRIBUTED BY (id);

SET optimizer TO off;

-- j->jointype == JOIN_LASJ_NOTIN
SELECT
  t1.*
FROM
  A t1
WHERE
  t1.id NOT IN (SELECT id FROM A WHERE id != 4)
  AND
  t1.tt = (SELECT MIN(tt) FROM A WHERE id = t1.id);

-- j->jointype == JOIN_ANTI
SELECT
  t1.*
FROM
  A t1
WHERE
  NOT EXISTS (SELECT id FROM A WHERE id != 4 and id = t1.id)
  AND t1.tt = (SELECT MIN(tt) FROM A WHERE id = t1.id);
```

Add these two jointype into the Assert(). I made several simple
tests, results are ok. However, same as the code comment said, i
am also unsure if postponing quals past anti-join/lasj_notin-join
is always semantically correct.
Copy link
Contributor

@kainwen kainwen left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for your contribution @dh-cloud .

I've force push to add a test case in the commit message.

@kainwen kainwen merged commit c857853 into greenplum-db:main Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community PR from outside VMware topic: planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants