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

[YSQL] Avoid choosing YB Bitmap Scans all the time with CBO #21479

Closed
1 task done
timothy-e opened this issue Mar 14, 2024 · 0 comments
Closed
1 task done

[YSQL] Avoid choosing YB Bitmap Scans all the time with CBO #21479

timothy-e opened this issue Mar 14, 2024 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@timothy-e
Copy link
Contributor

timothy-e commented Mar 14, 2024

Jira Link: DB-10362

Description

YB Bitmap Scans don't have a special CBO costing yet, so they default to the PG costing. This is a much lower value than other CBO costs, so it is often selected when it shouldn't be.

This should be a temporary fix to unblock master until #20573 is addressed.

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@timothy-e timothy-e added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Mar 14, 2024
@timothy-e timothy-e self-assigned this Mar 14, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue and removed status/awaiting-triage Issue awaiting triage labels Mar 14, 2024
timothy-e added a commit that referenced this issue Mar 15, 2024
Summary:
To avoid any regressions with CBO on, disable bitmap scan
until we have better defaults for costing Bitmap Scans with
CBO. Tracked by #20573
Jira: DB-10362

Test Plan:
Jenkins: urgent, test regex: .*TestPgRegress.*

```
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressContribPgTrgm'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressThirdPartyExtensionsPgHintPlan'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPartitions'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPgSelect'
```

Reviewers: smishra

Reviewed By: smishra

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D33161
timothy-e added a commit that referenced this issue Mar 15, 2024
Summary:
Original commit: b038851 / D33161
To avoid any regressions with CBO on, disable bitmap scan
until we have better defaults for costing Bitmap Scans with
CBO. Tracked by #20573
Jira: DB-10362

Test Plan:
Jenkins: urgent, test regex: .*TestPgRegress.*

```
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressContribPgTrgm'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressThirdPartyExtensionsPgHintPlan'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPartitions'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPgSelect'
```

Reviewers: smishra

Reviewed By: smishra

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D33187
asrinivasanyb pushed a commit to asrinivasanyb/yugabyte-db that referenced this issue Mar 18, 2024
Summary:
To avoid any regressions with CBO on, disable bitmap scan
until we have better defaults for costing Bitmap Scans with
CBO. Tracked by yugabyte#20573
Jira: DB-10362

Test Plan:
Jenkins: urgent, test regex: .*TestPgRegress.*

```
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressContribPgTrgm'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressThirdPartyExtensionsPgHintPlan'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPartitions'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPgSelect'
```

Reviewers: smishra

Reviewed By: smishra

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D33161
jasonyb pushed a commit that referenced this issue May 8, 2024
…ter-merge

Merge YB master commit b038851 titled

    [#21479] YSQL: Disable bitmap scan by default

and committed 2024-03-14T22:28:20-04:00 into YB pg15.  This includes the
initial bitmap scan commit a81450b.

YB pg15 initial merge refers to
55782d5.

- yb_scan.c:
  - YbBindScanKeys: YB pg15 initial merge changes ybScan->relation to
    ((TableScanDesc)ybScan)->rs_rd.  It also adds an extra newline
    unnecessarily.  YB master a81450b
    changes to use scan_plan->target_relation. Although it is not
    explained, that change appears to be more fitting in this case
    because it is paired with bind_key_attnums which is based off of
    target_relation (ybScan->relation and rs_rd are always referring to
    the table whereas target_relation could refer to either the table or
    index depending on the type of scan).  So take YB master's version.
  - ybcSetupTargets: YB pg15 8edf353
    touches comment that YB master
    a81450b also touches.  Apply both.
- explain.c:
  - function declarations: YB master
    a81450b updates show_yb_rpc_stats
    parameters.  YB pg15 changes adjacent lines.  Trivial resolution.
- executor/Makefile:
  - OBJS: YB master a81450b adds
    nodeYbBitmapTablescan.o to the list.  YB pg15 reformats it.  Take
    the opportunity to put YB objects to the top of the list for less
    conflicts with upstream PG.
- nodes/Makefile:
  - OBJS: YB master a81450b adds
    ybtidbitmap.o to the list.  YB pg15 reformats it.
- nodes/README:
  - FILES IN THIS DIRECTORY: upstream PG
    639a86e36aaecb84faaf941dcd0b183ba0aba9e9 changes "Value" to "value",
    and YB master a81450b adds an
    adjacent line.  Trivial resolution.
- indxpath.c:
  - function declarations: YB master
    a81450b adds
    yb_bitmap_scan_cost_est next to lines that YB pg15 touches.  Trivial
    resolution.
  - choose_bitmap_and: adjacent lines conflict with upstream PG
    5ee190f8ec37c1bbfb3061e18304e155d600bc8e.
- yb_uniqkeys.c:
  - yb_is_const_clause_for_distinct_pushdown: conflict between YB pg15
    40e68e2 and YB master
    a81450b.  Take YB pg15's version
    which makes YB master's change obsolete.
- createplan.c:
  - function declarations: adjacent line conflict between YB pg15
    changing adjacent indentation and YB master
    a81450b adding new functions
    YbBitmapTableScan and make_yb_bitmap_tablescan.
- plancat.c:
  - get_relation_info: adjacent line conflict between YB pg15 defining
    info->yb_amhasgetbitmap and upstream PG
    bfbcad478f05794e5e7ea1339e62a1d258c99b6c and
    660b89928d18386de7755565c008439ae75d1218.
- execnodes.h:
  - BitmapIndexScanState: adjacent line conflict between upstream PG
    0944ec54de389b4b8a471ca1f40f1b9d81de1f30 and YB master
    a81450b.
- nodes.h:
  - NodeTag: YB pg15 initial merge removes an empty line at the end of
    NodeTag enum definition, and YB master
    a81450b adds T_YbTIDBitmap there.
    Initial merge is at fault for doing something unrelated to merge, so
    take YB master's version.
- cost.h:
  - function declarations: adjacent line conflict between YB pg15
    indentation changes and YB master
    a81450b adding
    cost_yb_bitmap_table_scan.
- pathnode.h:
  - function declarations: adjacent line conflict between YB pg15
    indentation changes and YB master
    a81450b adding
    create_yb_bitmap_table_path.
- sysviews.out:
  - YB master a81450b should not have
    touched this file at all.  Discard all YB master changes.
- pg_hint_plan.out:
  - Take YB master a81450b's comments
    as they don't have typos.
- pg_hint_plan.sql:
  - (same)
- pg_dml.cc:
  - PgDml::has_secondary_index_with_doc_op: YB master
    a81450b adds this function.
    Adjacent line conflict with YB pg15
    5e6edbf which removes
    PgDml::has_system_targets.
- pg_dml.h:
  - has_secondary_index_with_doc_op: (same)
- indexam.c:
  - yb_index_getbitmap: upstream PG
    346ed70b0ad7efc574711a97812692dab4542712 changes rd_amroutine to
    rd_indam.  YB master a81450b adds
    usage of rd_amroutine.  Change it to rd_indam.
- yb_lsm.c:
  - ybcgetbitmap: YB pg15 initial merge changes ybScan->relation to
    ybScan->rs_base.rs_rd (or ((TableScanDesc)ybScan)->rs_rd).  This is
    also equivalent to scan->heapRelation (if scan is available), so
    substitute ybscan->relation added by YB master
    a81450b with scan->heapRelation in
    this case.  Also take the opportunity to wrap to fit the 80-char
    limit.
- nodeBitmapIndexscan.c:
  - ExecEndBitmapIndexScan: upstream PG
    9ddef36278a9f676c07d0b4d9f33fa22e48ce3b5 removes
    ExecCloseScanRelation, so get rid of the logic introduced by YB
    master.
- setrefs.c:
  - set_plan_refs: upstream PG 41efb8340877e8ffd0023bb6b2ef22ffd1ca014d
    adds param to fix_scan_list, and YB master
    a81450b adds set_plan_refs calls.
    Easy merge.
- nodeYbBitmapTablescan.c:
  - includes:
    - Upstream PG c91560defc57f89f7e88632ea14ae77b5cec78ee removes
      include utils/tqual.h from nodeBitmapHeapscan.c, so do the same
      here.
    - Upstream PG c2fe139c201c48f1133e9fbea2dd99b8efe2fadd adds include
      access/tableam.h to nodeBitmapHeapscan.c, so do the same here.
    - Include access/yb_scan.h because of new ybcBeginScan calls (see
      below).
    - Include utils/snapmgr.h because IsMVCCSnapshot used to be defined
      in tqual.h (removed as described above) but is now defined in
      snapmgr.h.
  - YbBitmapTableNext: mostly imitate YbSeqNext:
    - tsdesc: upstream PG c2fe139c201c48f1133e9fbea2dd99b8efe2fadd
      modifies BitmapHeapNext to change HeapScanDesc scan to
      TableScanDesc, so do similarly here, naming it like in YbSeqNext.
    - estate: a convenience variable used as in YbSeqNext.
    - ybcBeginScan: call ybcBeginScan as in YbSeqNext and set tsdesc to
      ybScan returned by it.  Aggregate pushdown is not (yet?) supported
      for bitmap scan nodes, so pass NULL for aggrefs.
    - Since parallel scan is not yet supported by bitmap scan, skip
      setting pscan.
  - ExecReScanYbBitmapTableScan: merge changes of both
    ExecReScanYbSeqScan and ExecReScanBitmapHeapScan.  This function is
    mainly following ExecReScanBitmapHeapScan, but adjust it similar to
    how ExecReScanYbSeqScan adjusts to ExecReScanSeqScan by swapping
    table_rescan for
    - ybc_heap_endscan and
    - setting node->ss.ss_currentScanDesc to NULL
    Furthermore, an if (tsdesc) check is needed, and the reasoning is
    explained in code comment there.
  - ExecEndYbBitmapTableScan:
    - Similar to ExecEndBitmapIndexScan, get rid ExecCloseScanRelation.
      Furthermore, remove now unused relation local variable.
    - Change to use tsdesc as in ExecEndYbSeqScan.  Also change
      heap_endscan/table_endscan to ybc_heap_endscan as in
      ExecEndYbSeqScan.
  - ExecInitYbBitmapTableScan:
    - Pass TTSOpsVirtual to ExecInitScanTupleSlot as done in
      ExecInitYbSeqScan.
    - For any changes involivng ss_currentScanDesc, drop them because
      they those lines are removed by future YB master
      03fd372 (and they cause
      compilation failure ‘struct TableScanDescData’ has no member named
      ‘ybscan’).
- yb_bitmap_scan_joins.out:
  - Upstream PG 1a8d5afb0dfc5d0dcc6eda0656a34cb1f0cf0bdf causes some
    operators to commute their operands, so update EXPLAIN output.
- version.txt:
  - Conflict between YB pg15 26ba608
    and YB master 2a4cf1e.
- namespace.c:
  - function declarations: adjacent line conflict between YB master
    bde7acd and upstream PG
    8255c7a5eeba8f1a38b7a431c04909bde4f5e67d and
    e56bce5d43789cce95d099554ae9593ada92b3b7.
- execReplication.c:
  - CheckCmdReplicaIdentity: upstream PG
    52e4f0cd472d39d07732b99559989ea3b615be78 and
    923def9a533a7d986acfb524139d8b9e5466d0a5 split up a single if
    condition to two.  YB master
    4da9d86 adds a third part to that
    if condition.  Split it out as well.
- nodeIndexonlyscan.c:
  - yb_agg_pushdown_init_scan_slot: YB master
    6fff498 and YB pg15 cherry-pick
    b4e4a16 conflict.  Take pg15's
    version.
- nodeIndexscan.c:
  - yb_agg_pushdown_init_scan_slot: (same).
- proto.c:
  - logicalrep_write_insert: upstream PG
    1088729e84cc382270c592ac8c57c323836f40ca removes an assert that YB
    master 4da9d86 adds to.  Discard
    the YB change.
- kwlist.h:
  - YB master 4da9d86 adds "change"
    keyword.  Upstream PG 06a7c3154f5bfad65549810cc84f0e3a77b408bf adds
    extra argument for bare label.  "change" is likely fine to be part
    of a SELECT list without requiring "AS", so mark it as a bare label
    for now and we can come back and change it to AS_LABEL if that ends
    up causing grammar conflict issues.
- yb_triggers.out:
  - YB master bde7acd does some \o
    replace_temp_schema_name trickery on some queries where YB pg15
    e8b60ce changes "PROCEDURE" to
    "FUNCITON".  Apply both.
- pg_hint_plan/expected/init.out:
  - Adjacent line conflict between YB master
    b038851 changing enable_bitmapscan
    default to off, which is shown in test output, and upstream
    pg_hint_plan 454f72a adding
    adjacent enable_async_append.
- gram.y:
  - bare_label_keyword: add CHANGE keyword to list, mostly following
    what happened to the adjacent CHAIN keyword.  (See also kwlist.h
    explanation.)
- guc.c:
  - yb_enable_replica_identity: YB master
    4da9d86 adds this new GUC with
    "REPLICATION" category, which was removed by upstream PG.  Switch
    to "REPLICATION_SENDING".
- selfuncs.c:
  - gincost_pattern: nontrivial conflict between upstream PG
    4b754d6c16e16cc1a1adf12ab0f48603069a0efd and YB master
    4548b2a.  Upstream PG changes

        if (elemcounts.haveFullScan)

    to

        if (elemcounts.attHasFullScan[indexcol] &&
        	!elemcounts.attHasNormalScan[indexcol])

    and YB master tries to forcibly enter this condition, so adjust
    settings accordingly.
  - gincostestimate: YB master 4548b2a
    adds usage of qinfos, but upstream PG
    e89f14e2bb9f7c392c4c85a53ab5a13ea2aed83d refactors and removes that.
    Use path->indexclauses instead.
- yb_ybgin_cost_estimate.out:
  - Apparently, SELECT * FROM jsonbs WHERE j @? '$.aaa[*] ? (@ > 2)';
    now can use storage filter, and with some small manual tests, it
    appears work, so update the output.
- yb_pg_pg_trgm.out:
  - YB master b038851 adds SET
    enable_bitmapscan = on; to counteract setting the default to false,
    but YB pg15 eb19fa3 adds more
    queries above that setting that expect to use bitmap scan.  Move
    this line higher to capture those queries as well.
  - YB master 4548b2a improves cost
    estimates for ybgin index scans so that unsupported scans are
    avoided.  YB pg15 eb19fa3 adds a
    query

        select count(*) from test_trgm where t like '%99%' and t like '%qwerty%';

    that expects to fail on unsupported scan but now passes.  Change the
    expectation accordingly.  The test actually appears to be trying to
    run gin (bitmap) index scan before and sequential scan after, but
    ybgin currently supports regular index scan and doesn't support
    bitmap index scan, so it isn't really doing what the original author
    intended.  This should be followed up on.
- BasePgSQLTest.java:
  - cleanUpAfter: reset seqscan/bitmapscan GUCs because the following
    queries such as those in cleanUpCustomEntities may do bitmap scans
    otherwise, and there is a known bug DB-11127 for bitmap scans in YB
    pg15.  Java test
    org.yb.pgsql.TestPgRegressHashInQueries#testInQueryBatchingNestLoopHashKey
    specifically needs this.
- yb_bitmap_scans.out:
  - SELECT * FROM tenk1 WHERE thousand < 500 OR hundred >= 75:
    introduced by YB master a81450b,
    expected storage table read requests number decreases by one in YB
    pg15.  Not sure why, but that's a good thing, so go along with it.
  - UPDATE tenk3 SET unique2 = NULL WHERE unique2 < 100 OR unique1 < 10:
    introduced by YB master a81450b,
    the explain cost estimate on master is rows=10 but rows=0 on pg15.
    See upstream PG f0f13a3a08b2757997410f3a1c38bdc22973c525.
  - DELETE FROM tenk3 WHERE unique2 IS NULL OR unique1 < 1000: (same)
  - (There are some EXPLAIN cost differences, but those should go away
    after YB master 7b418b8.)
  - (System table scan fails due to bug DB-11127.)
- yb_pg_box.out:
  - YB pg15 4a5799c acknowledges some
    temp index scans fail due to an issue on master.  That issue is
    fixed by YB master 3a4cb79 that is
    part of this merge.  Update to the better expected output.
- yb_pg_point.out: (same)
- yb_pg_point.sql: (same)
- pg15_tests/test_yb_bitmap_scans.sh:
  - Add this shell test for coverage of the new Java test
    TestPgRegressYbBitmapScans.
- yb_pg_replica_identity.out:
  - \d+ test_replica_identity: YB master
    4da9d86 introduces this test.  The
    order of indexes output changes between PG 11.2 and PG 15.2
    (according to how the original test is ordered), so reorder the same
    way in this ported test.
jasonyb pushed a commit that referenced this issue May 8, 2024
…a311fdc' into pg15

Summary:
Merge YB master commit b038851 titled

    [#21479] YSQL: Disable bitmap scan by default

and committed 2024-03-14T22:28:20-04:00 into YB pg15.  This includes the
initial bitmap scan commit a81450b.

YB pg15 initial merge refers to
55782d5.

- yb_scan.c:
  - YbBindScanKeys: YB pg15 initial merge changes ybScan->relation to
    ((TableScanDesc)ybScan)->rs_rd.  It also adds an extra newline
    unnecessarily.  YB master a81450b
    changes to use scan_plan->target_relation. Although it is not
    explained, that change appears to be more fitting in this case
    because it is paired with bind_key_attnums which is based off of
    target_relation (ybScan->relation and rs_rd are always referring to
    the table whereas target_relation could refer to either the table or
    index depending on the type of scan).  So take YB master's version.
  - ybcSetupTargets: YB pg15 8edf353
    touches comment that YB master
    a81450b also touches.  Apply both.
- explain.c:
  - function declarations: YB master
    a81450b updates show_yb_rpc_stats
    parameters.  YB pg15 changes adjacent lines.  Trivial resolution.
- executor/Makefile:
  - OBJS: YB master a81450b adds
    nodeYbBitmapTablescan.o to the list.  YB pg15 reformats it.  Take
    the opportunity to put YB objects to the top of the list for less
    conflicts with upstream PG.
- nodes/Makefile:
  - OBJS: YB master a81450b adds
    ybtidbitmap.o to the list.  YB pg15 reformats it.
- nodes/README:
  - FILES IN THIS DIRECTORY: upstream PG
    639a86e36aaecb84faaf941dcd0b183ba0aba9e9 changes "Value" to "value",
    and YB master a81450b adds an
    adjacent line.  Trivial resolution.
- indxpath.c:
  - function declarations: YB master
    a81450b adds
    yb_bitmap_scan_cost_est next to lines that YB pg15 touches.  Trivial
    resolution.
  - choose_bitmap_and: adjacent lines conflict with upstream PG
    5ee190f8ec37c1bbfb3061e18304e155d600bc8e.
- yb_uniqkeys.c:
  - yb_is_const_clause_for_distinct_pushdown: conflict between YB pg15
    40e68e2 and YB master
    a81450b.  Take YB pg15's version
    which makes YB master's change obsolete.
- createplan.c:
  - function declarations: adjacent line conflict between YB pg15
    changing adjacent indentation and YB master
    a81450b adding new functions
    YbBitmapTableScan and make_yb_bitmap_tablescan.
- plancat.c:
  - get_relation_info: adjacent line conflict between YB pg15 defining
    info->yb_amhasgetbitmap and upstream PG
    bfbcad478f05794e5e7ea1339e62a1d258c99b6c and
    660b89928d18386de7755565c008439ae75d1218.
- execnodes.h:
  - BitmapIndexScanState: adjacent line conflict between upstream PG
    0944ec54de389b4b8a471ca1f40f1b9d81de1f30 and YB master
    a81450b.
- nodes.h:
  - NodeTag: YB pg15 initial merge removes an empty line at the end of
    NodeTag enum definition, and YB master
    a81450b adds T_YbTIDBitmap there.
    Initial merge is at fault for doing something unrelated to merge, so
    take YB master's version.
- cost.h:
  - function declarations: adjacent line conflict between YB pg15
    indentation changes and YB master
    a81450b adding
    cost_yb_bitmap_table_scan.
- pathnode.h:
  - function declarations: adjacent line conflict between YB pg15
    indentation changes and YB master
    a81450b adding
    create_yb_bitmap_table_path.
- sysviews.out:
  - YB master a81450b should not have
    touched this file at all.  Discard all YB master changes.
- pg_hint_plan.out:
  - Take YB master a81450b's comments
    as they don't have typos.
- pg_hint_plan.sql:
  - (same)
- pg_dml.cc:
  - PgDml::has_secondary_index_with_doc_op: YB master
    a81450b adds this function.
    Adjacent line conflict with YB pg15
    5e6edbf which removes
    PgDml::has_system_targets.
- pg_dml.h:
  - has_secondary_index_with_doc_op: (same)
- indexam.c:
  - yb_index_getbitmap: upstream PG
    346ed70b0ad7efc574711a97812692dab4542712 changes rd_amroutine to
    rd_indam.  YB master a81450b adds
    usage of rd_amroutine.  Change it to rd_indam.
- yb_lsm.c:
  - ybcgetbitmap: YB pg15 initial merge changes ybScan->relation to
    ybScan->rs_base.rs_rd (or ((TableScanDesc)ybScan)->rs_rd).  This is
    also equivalent to scan->heapRelation (if scan is available), so
    substitute ybscan->relation added by YB master
    a81450b with scan->heapRelation in
    this case.  Also take the opportunity to wrap to fit the 80-char
    limit.
- nodeBitmapIndexscan.c:
  - ExecEndBitmapIndexScan: upstream PG
    9ddef36278a9f676c07d0b4d9f33fa22e48ce3b5 removes
    ExecCloseScanRelation, so get rid of the logic introduced by YB
    master.
- setrefs.c:
  - set_plan_refs: upstream PG 41efb8340877e8ffd0023bb6b2ef22ffd1ca014d
    adds param to fix_scan_list, and YB master
    a81450b adds set_plan_refs calls.
    Easy merge.
- nodeYbBitmapTablescan.c:
  - includes:
    - Upstream PG c91560defc57f89f7e88632ea14ae77b5cec78ee removes
      include utils/tqual.h from nodeBitmapHeapscan.c, so do the same
      here.
    - Upstream PG c2fe139c201c48f1133e9fbea2dd99b8efe2fadd adds include
      access/tableam.h to nodeBitmapHeapscan.c, so do the same here.
    - Include access/yb_scan.h because of new ybcBeginScan calls (see
      below).
    - Include utils/snapmgr.h because IsMVCCSnapshot used to be defined
      in tqual.h (removed as described above) but is now defined in
      snapmgr.h.
  - YbBitmapTableNext: mostly imitate YbSeqNext:
    - tsdesc: upstream PG c2fe139c201c48f1133e9fbea2dd99b8efe2fadd
      modifies BitmapHeapNext to change HeapScanDesc scan to
      TableScanDesc, so do similarly here, naming it like in YbSeqNext.
    - estate: a convenience variable used as in YbSeqNext.
    - ybcBeginScan: call ybcBeginScan as in YbSeqNext and set tsdesc to
      ybScan returned by it.  Aggregate pushdown is not (yet?) supported
      for bitmap scan nodes, so pass NULL for aggrefs.
    - Since parallel scan is not yet supported by bitmap scan, skip
      setting pscan.
  - ExecReScanYbBitmapTableScan: merge changes of both
    ExecReScanYbSeqScan and ExecReScanBitmapHeapScan.  This function is
    mainly following ExecReScanBitmapHeapScan, but adjust it similar to
    how ExecReScanYbSeqScan adjusts to ExecReScanSeqScan by swapping
    table_rescan for
    - ybc_heap_endscan and
    - setting node->ss.ss_currentScanDesc to NULL
    Furthermore, an if (tsdesc) check is needed, and the reasoning is
    explained in code comment there.
  - ExecEndYbBitmapTableScan:
    - Similar to ExecEndBitmapIndexScan, get rid ExecCloseScanRelation.
      Furthermore, remove now unused relation local variable.
    - Change to use tsdesc as in ExecEndYbSeqScan.  Also change
      heap_endscan/table_endscan to ybc_heap_endscan as in
      ExecEndYbSeqScan.
  - ExecInitYbBitmapTableScan:
    - Pass TTSOpsVirtual to ExecInitScanTupleSlot as done in
      ExecInitYbSeqScan.
    - For any changes involivng ss_currentScanDesc, drop them because
      they those lines are removed by future YB master
      03fd372 (and they cause
      compilation failure ‘struct TableScanDescData’ has no member named
      ‘ybscan’).
- yb_bitmap_scan_joins.out:
  - Upstream PG 1a8d5afb0dfc5d0dcc6eda0656a34cb1f0cf0bdf causes some
    operators to commute their operands, so update EXPLAIN output.
- version.txt:
  - Conflict between YB pg15 26ba608
    and YB master 2a4cf1e.
- namespace.c:
  - function declarations: adjacent line conflict between YB master
    bde7acd and upstream PG
    8255c7a5eeba8f1a38b7a431c04909bde4f5e67d and
    e56bce5d43789cce95d099554ae9593ada92b3b7.
- execReplication.c:
  - CheckCmdReplicaIdentity: upstream PG
    52e4f0cd472d39d07732b99559989ea3b615be78 and
    923def9a533a7d986acfb524139d8b9e5466d0a5 split up a single if
    condition to two.  YB master
    4da9d86 adds a third part to that
    if condition.  Split it out as well.
- nodeIndexonlyscan.c:
  - yb_agg_pushdown_init_scan_slot: YB master
    6fff498 and YB pg15 cherry-pick
    b4e4a16 conflict.  Take pg15's
    version.
- nodeIndexscan.c:
  - yb_agg_pushdown_init_scan_slot: (same).
- proto.c:
  - logicalrep_write_insert: upstream PG
    1088729e84cc382270c592ac8c57c323836f40ca removes an assert that YB
    master 4da9d86 adds to.  Discard
    the YB change.
- kwlist.h:
  - YB master 4da9d86 adds "change"
    keyword.  Upstream PG 06a7c3154f5bfad65549810cc84f0e3a77b408bf adds
    extra argument for bare label.  "change" is likely fine to be part
    of a SELECT list without requiring "AS", so mark it as a bare label
    for now and we can come back and change it to AS_LABEL if that ends
    up causing grammar conflict issues.
- yb_triggers.out:
  - YB master bde7acd does some \o
    replace_temp_schema_name trickery on some queries where YB pg15
    e8b60ce changes "PROCEDURE" to
    "FUNCITON".  Apply both.
- pg_hint_plan/expected/init.out:
  - Adjacent line conflict between YB master
    b038851 changing enable_bitmapscan
    default to off, which is shown in test output, and upstream
    pg_hint_plan 454f72a adding
    adjacent enable_async_append.
- gram.y:
  - bare_label_keyword: add CHANGE keyword to list, mostly following
    what happened to the adjacent CHAIN keyword.  (See also kwlist.h
    explanation.)
- guc.c:
  - yb_enable_replica_identity: YB master
    4da9d86 adds this new GUC with
    "REPLICATION" category, which was removed by upstream PG.  Switch
    to "REPLICATION_SENDING".
- selfuncs.c:
  - gincost_pattern: nontrivial conflict between upstream PG
    4b754d6c16e16cc1a1adf12ab0f48603069a0efd and YB master
    4548b2a.  Upstream PG changes

        if (elemcounts.haveFullScan)

    to

        if (elemcounts.attHasFullScan[indexcol] &&
        	!elemcounts.attHasNormalScan[indexcol])

    and YB master tries to forcibly enter this condition, so adjust
    settings accordingly.
  - gincostestimate: YB master 4548b2a
    adds usage of qinfos, but upstream PG
    e89f14e2bb9f7c392c4c85a53ab5a13ea2aed83d refactors and removes that.
    Use path->indexclauses instead.
- yb_ybgin_cost_estimate.out:
  - Apparently, SELECT * FROM jsonbs WHERE j @? '$.aaa[*] ? (@ > 2)';
    now can use storage filter, and with some small manual tests, it
    appears work, so update the output.
- yb_pg_pg_trgm.out:
  - YB master b038851 adds SET
    enable_bitmapscan = on; to counteract setting the default to false,
    but YB pg15 eb19fa3 adds more
    queries above that setting that expect to use bitmap scan.  Move
    this line higher to capture those queries as well.
  - YB master 4548b2a improves cost
    estimates for ybgin index scans so that unsupported scans are
    avoided.  YB pg15 eb19fa3 adds a
    query

        select count(*) from test_trgm where t like '%99%' and t like '%qwerty%';

    that expects to fail on unsupported scan but now passes.  Change the
    expectation accordingly.  The test actually appears to be trying to
    run gin (bitmap) index scan before and sequential scan after, but
    ybgin currently supports regular index scan and doesn't support
    bitmap index scan, so it isn't really doing what the original author
    intended.  This should be followed up on.
- BasePgSQLTest.java:
  - cleanUpAfter: reset seqscan/bitmapscan GUCs because the following
    queries such as those in cleanUpCustomEntities may do bitmap scans
    otherwise, and there is a known bug DB-11127 for bitmap scans in YB
    pg15.  Java test
    org.yb.pgsql.TestPgRegressHashInQueries#testInQueryBatchingNestLoopHashKey
    specifically needs this.
- yb_bitmap_scans.out:
  - SELECT * FROM tenk1 WHERE thousand < 500 OR hundred >= 75:
    introduced by YB master a81450b,
    expected storage table read requests number decreases by one in YB
    pg15.  Not sure why, but that's a good thing, so go along with it.
  - UPDATE tenk3 SET unique2 = NULL WHERE unique2 < 100 OR unique1 < 10:
    introduced by YB master a81450b,
    the explain cost estimate on master is rows=10 but rows=0 on pg15.
    See upstream PG f0f13a3a08b2757997410f3a1c38bdc22973c525.
  - DELETE FROM tenk3 WHERE unique2 IS NULL OR unique1 < 1000: (same)
  - (There are some EXPLAIN cost differences, but those should go away
    after YB master 7b418b8.)
  - (System table scan fails due to bug DB-11127.)
- yb_pg_box.out:
  - YB pg15 4a5799c acknowledges some
    temp index scans fail due to an issue on master.  That issue is
    fixed by YB master 3a4cb79 that is
    part of this merge.  Update to the better expected output.
- yb_pg_point.out: (same)
- yb_pg_point.sql: (same)
- pg15_tests/test_yb_bitmap_scans.sh:
  - Add this shell test for coverage of the new Java test
    TestPgRegressYbBitmapScans.
- yb_pg_replica_identity.out:
  - \d+ test_replica_identity: YB master
    4da9d86 introduces this test.  The
    order of indexes output changes between PG 11.2 and PG 15.2
    (according to how the original test is ordered), so reorder the same
    way in this ported test.

Test Plan:
On Almalinux 8:

    #!/usr/bin/env bash
    set -eu
    ./yb_build.sh fastdebug --gcc11
    pg15_tests/run_tests.sh
    for _ in {1..50}; do pg15_tests/get_shell_test_specs.sh | grep bitmap; done | pg15_tests/run_tests.sh

Got the following results on Almalinux 8, fastdebug, gcc11:

    1	2024-05-08T13:12:42-07:00	JAVA	org.yb.pgsql.TestPgRegressYbFetchLimits
    0	2024-05-08T13:15:55-07:00	JAVA	org.yb.pgsql.TestPgBatch
    1	2024-05-08T13:17:03-07:00	JAVA	org.yb.pgsql.TestPgRegressParallel
    0	2024-05-08T13:17:16-07:00	pgwrapper_pg_mini-test	PgMiniTest.AlterTableWithReplicaIdentity
    1	2024-05-08T13:18:12-07:00	JAVA	org.yb.pgsql.TestPgAlterTable
    0	2024-05-08T13:18:48-07:00	JAVA	org.yb.pgsql.TestPgRegressReplicaIdentity
    0	2024-05-08T13:19:07-07:00	pgwrapper_pg_ddl_atomicity-test	PgDdlAtomicitySanityTest.AddReplicaIdentityTest
    0	2024-05-08T13:19:57-07:00	tools_yb-backup-cross-feature-test	YBBackupTest.TestReplicaIdentityAfterRestore
    0	2024-05-08T13:20:27-07:00	tools_yb-admin-snapshot-schedule-test	YbAdminSnapshotScheduleTest.CloneYcql
    0	2024-05-08T13:20:42-07:00	pgwrapper_pg_shared_mem-test	PgSharedMemTest.Batches
    0	2024-05-08T13:21:33-07:00	pgwrapper_pg_packed_row-test	PackingVersion/PgPackedRowTest.RestorePITRSnapshotAfterOldSchemaGC/*
    0	2024-05-08T13:22:18-07:00	tools_yb-backup-cross-feature-test	PackedRows/YBBackupTestWithPackedRowsAndColocation.RestoreBackupAfterOldSchemaGC/1
    0	2024-05-08T13:25:26-07:00	tools_yb-admin-snapshot-schedule-test	Colocation/YbAdminSnapshotScheduleTestWithYsqlParam.PgsqlCreateIndex/*
    0	2024-05-08T13:25:46-07:00	pgwrapper_pg_libpq-test	PgLibPqTest.TempTableMultiNodeNamespaceConflict
    1	2024-05-08T13:26:39-07:00	JAVA	org.yb.pgsql.TestPgRegressArrays
    0	2024-05-08T13:26:47-07:00	client_snapshot-schedule-test	CloneFromScheduleTest.CloneWithNoSchedule
    1	2024-05-08T13:35:10-07:00	JAVA	org.yb.pgsql.TestPgRegressPartitions
    0	2024-05-08T13:35:52-07:00	JAVA	org.yb.pgsql.TestPgRegressPgSelect

Co-authored-by: timothy-e <[email protected]>
Jenkins: urgent, rebase: pg15

Reviewers: telgersma, xCluster, hsunder

Reviewed By: telgersma

Subscribers: ybase, ycdcxcluster, tfoucher, yql

Differential Revision: https://phorge.dev.yugabyte.com/D34762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

2 participants