pgwire/pgtest: make the tests work both on pg and crdb

[knz]

There was some accumulated incompatibilities in the test files.
This patch fixes it, and in the process of doing so discovers
two new bugs (#49639 and #49640).

Summary of changes to the DSL:

• the new only directive skips an entire test file if the db
  does not match (used for the crdb-specific portal bug test file)

• the new flags crdb_only and noncrdb_only skip over a
  test directive if the db does not match.

• the new flags ignore_table_oids and ignore_type_oids replace
  the corresponding OIDs in the RowDescription message by 0
  prior to comparing with the expected value.

Release note: None

[cockroach-teamcity]

This change is 

[rohany]

I wonder if it would be simpler (and feasible in the implementation) instead of have crdb_only and no_crdb on each command to have begin crdb_only and end_crdb_only blocks (same with no_crdb).

[knz]

I wonder if it would be simpler (and feasible in the implementation) instead of have crdb_only and no_crdb on each command to have begin crdb_only and end_crdb_only blocks (same with no_crdb).

The datadriven engine does not support this yet unfortunately. Unless you have a suggestion to trick it into it?

[rohany]

I see, I wasn't sure what the datadriven library could do. LGTM

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz, @otan, @rafiss, and @rohany)

---

pkg/sql/pgwire/testdata/pgtest/row_description, line 75 at r1 (raw file):

----

until ignore_table_oids

I disagree with adding this directive here -- this test was created to verify that the Table OIDs get populated. See #48417

I'm not sure what to do about the fact that the OIDs differ between CRDB and Postgres. But if I had to pick, then I think the test should be run with crdb_only

---

pkg/sql/pgwire/testdata/pgtest/row_description, line 171 at r1 (raw file):

# The following discrepancy is another bug.
# See: https://github.com/cockroachdb/cockroach/issues/49640

covered by #49215 as well

[rafiss]

Thanks for filing the new issues, and for unifying these tests. Very much in favor, except for the changes to the row_description test.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz, @otan, @rafiss, and @rohany)

[knz]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan, @rafiss, and @rohany)

---

pkg/sql/pgwire/testdata/pgtest/row_description, line 75 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

I disagree with adding this directive here -- this test was created to verify that the Table OIDs get populated. See #48417

I'm not sure what to do about the fact that the OIDs differ between CRDB and Postgres. But if I had to pick, then I think the test should be run with crdb_only

Done.

---

pkg/sql/pgwire/testdata/pgtest/row_description, line 171 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

covered by #49215 as well

Changed the ref.

[knz]

friendly ping

[rafiss]

looks good to me!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan, @rafiss, and @rohany)

[knz]

thanks!

bors r=rafiss,rohany

[craig]

Build succeeded

• GitHub CI (Cockroach)