Repository navigation
fix(metadata): tolerate NULL catalog scans on openGauss - #592
Closed
yimiaoxiehou wants to merge 2 commits into
Closed
yimiaoxiehou wants to merge 2 commits into
yimiaoxiehou wants to merge 2 commits into
Conversation
usql guards nullable catalog columns with COALESCE(x, '') because database/sql cannot convert NULL into a string or int. On openGauss running in Oracle-compatible mode (sql_compatibility=A) the empty string literal '' is itself NULL, so COALESCE(x, '') evaluates to COALESCE(x, NULL) and the NULL still reaches the scanner. Every metadata command aborts with: failed to list tables: sql: Scan error on column index 5, name "Description": converting NULL to string is unsupported An empty string cannot be represented at all under these semantics, so the fix has to happen on the scan side. Add nullStr/nullInt/nullBool/ nullBoolVal scanner targets that absorb NULL, and route every scan in the information_schema and postgres metadata readers through them. The SQL is unchanged, so the queries remain correct under both PostgreSQL and openGauss semantics. Also accept int64 for is_grantable: information_schema.yes_or_no is a domain over boolean in PostgreSQL, but openGauss reports it as a numeric YES/NO, which used to break \dp with "cannot scan int64 into bool". Verified against openGauss 6.0.3: \l \dn \dt \dt+ \di \d \d+ \dT \dtT \df \dp \dv \ds \z \dC \dD \dx \dew \deu \sf all pass, no Scan errors.
The NULL-tolerant scan targets must not be stricter than the plain destinations they replace, or they would introduce new failures on the other databases sharing this information_schema reader (MySQL, SQL Server, ...), which never had this problem. database/sql converts into *int by rendering the driver value with asString and parsing the result, so it accepts numeric-as-text, bool and every integer/unsigned/float kind. nullInt only handled int64, int32, float64 and []byte, so a driver returning a value as, say, a string or uint64 would now fail where it previously worked. Replicate asString (strconv + reflect) and parse from that, matching the standard library exactly while still absorbing NULL. nullStr and nullBool are unaffected: database/sql's *string branch also accepts only string and []byte, which is what they already did. Adds nullscan_test.go asserting the targets accept exactly the value types database/sql converts, and that NULL is absorbed. Note the test cannot run without a working Docker daemon: TestMain in this package unconditionally starts database containers.
Member
|
@yimiaoxiehou there will not be any further releases on the If openGauss needs its own metadata model, please point a coding agent at that repo and instruct (prompt) it to build a DIALECT for that database. If it connects using the PG driver, it would be another "flavor" of the PostgreSQL DIALECT, and is well doxumented. I will make a point to add this, but otherwise rejecting this PR as significantly different plans are in motion already. That said, I do appreciate the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On openGauss, every metadata command fails outright:
\d,\df,\dpand\lfail the same way.Root cause
The readers guard nullable catalog columns with
COALESCE(x, '')becausedatabase/sqlcannot convert NULL into a string or int. That guard isuseless under openGauss' Oracle-compatible mode (
sql_compatibility=A),where the empty string literal
''is itself NULL:so
COALESCE(x, '')evaluates toCOALESCE(x, NULL)and the NULL stillreaches the scanner. An empty string cannot be represented at all under
these semantics, so the fix has to happen on the scan side rather than in
the SQL.
Fix
Add
nullStr/nullInt/nullBool/nullBoolValscan targets thatabsorb NULL, and route every scan in the
information_schemaandpostgresmetadata readers through them. The SQL is unchanged, so thequeries stay correct under both PostgreSQL and openGauss semantics.
nullIntdeliberately replicatesdatabase/sql'sasStringconversion soit accepts exactly the value types the plain
*intdestinations it replacesaccepted — otherwise a driver returning a number as text or as
uint64would start failing on the other databases sharing this reader (MySQL,
SQL Server, ...), which never had this problem.
nullStrandnullBoolalready matched the standard library, whose
*stringbranch accepts onlystringand[]byte.Also accept
int64foris_grantable:information_schema.yes_or_nois adomain over boolean in PostgreSQL but is reported as a numeric YES/NO by
openGauss, which otherwise broke
\dpwithcannot scan int64 into bool.Verification
Against openGauss 6.0.3 (
sql_compatibility=A), 23 metadata commands —\l \dn \dt \dt+ \di \d \d+ \dT \dtT \df \dp \dv \ds \z \dC \dD \dx \dew \deu \sfand pattern variants — all pass with no Scan errors. Output was alsochecked for correctness, not just absence of errors: comments, column
defaults and nullability all render correctly.
nullscan_test.goasserts the targets accept exactly the value typesdatabase/sqlconverts while absorbing NULL. Note that it cannot runwithout a working Docker daemon, as
TestMainin this packageunconditionally starts database containers.
Target branch
Opened against
release-21becausemainno longer contains this code:drivers/metadatahas been removed there (noobj_descriptionorCOALESCE(column_defaultremain anywhere in the repository). The0.21.x line is what still needs this fix.