Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a new Oracle-compatible SYS view Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
contrib/ivorysql_ora/sql/ora_sysview.sql (2)
134-137: Add test coverage fordata_typecolumn verification.The
all_cons_columnsview includes adata_typecolumn with Oracle-compatible type mappings, but the test only selectsowner,table_name,constraint_name,column_name, andposition. Consider adding verification of thedata_typecolumn to ensure type mappings work correctly.♻️ Proposed additional query to test data_type
-- Add this query to verify data_type mappings SELECT column_name, data_type FROM all_cons_columns WHERE owner = CURRENT_USER AND table_name = 't_data_types' ORDER BY column_name;As per coding guidelines: "Test SQL files. Ensure comprehensive coverage of features."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contrib/ivorysql_ora/sql/ora_sysview.sql` around lines 134 - 137, Add verification for the data_type column in the all_cons_columns test by extending the query coverage to select and assert data_type mappings; specifically, add a test query against the all_cons_columns view (same view name) that selects column_name and data_type for the test table t_data_types (use WHERE owner = CURRENT_USER AND table_name = 't_data_types' and ORDER BY column_name) so the test validates Oracle-compatible type mappings alongside the existing owner/table_name/constraint_name/column_name/position checks.
109-113: Partitioned table syntax is incompatible with PostgreSQL.The Oracle-style
PARTITION BY RANGE (id) (PARTITION p1 VALUES LESS THAN (100), ...)syntax is not supported in PostgreSQL. PostgreSQL uses a different approach with separate partition tables. The expected output shows this fails with a syntax error.If testing partitioned table constraints is desired, use PostgreSQL-compatible syntax:
♻️ Proposed fix using PostgreSQL partition syntax
-CREATE TABLE t_partitioned (id INT, data VARCHAR2(100)) PARTITION BY RANGE (id) ( - PARTITION p1 VALUES LESS THAN (100), - PARTITION p2 VALUES LESS THAN (200) -); -ALTER TABLE t_partitioned ADD CONSTRAINT pk_part PRIMARY KEY (id); +CREATE TABLE t_partitioned (id INT, data VARCHAR2(100)) PARTITION BY RANGE (id); +CREATE TABLE t_partitioned_p1 PARTITION OF t_partitioned FOR VALUES FROM (MINVALUE) TO (100); +CREATE TABLE t_partitioned_p2 PARTITION OF t_partitioned FOR VALUES FROM (100) TO (200); +ALTER TABLE t_partitioned ADD CONSTRAINT pk_part PRIMARY KEY (id);As per coding guidelines: "Test SQL files. Ensure comprehensive coverage of features."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contrib/ivorysql_ora/sql/ora_sysview.sql` around lines 109 - 113, The CREATE TABLE uses Oracle-style partition syntax which is invalid in PostgreSQL; replace the block that creates t_partitioned and its partitions and constraint by using PostgreSQL declarative partitioning: create the parent table t_partitioned WITH PARTITION BY RANGE on column id (so the partition key is id), then create separate child partitions (referencing t_partitioned) using FOR VALUES FROM/TO (or FOR VALUES LESS THAN equivalents), and finally add the primary key constraint pk_part on the parent table (ensuring it includes the partition key id); update references to t_partitioned and pk_part accordingly so the SQL is PostgreSQL-compatible.contrib/ivorysql_ora/expected/ora_sysview.out (1)
195-203: Known syntax errors are documented, but consider removing the failing statements.The expected output correctly documents that the Oracle-style partition syntax fails. However, including statements that are known to fail adds noise to the test output and may mask real failures.
Consider either:
- Removing the failing partition test entirely, or
- Fixing the syntax to use PostgreSQL-compatible partitioning (as suggested in the .sql file review).
As per coding guidelines: "Proper Oracle compatibility behavior" - the test should demonstrate working Oracle compatibility features.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contrib/ivorysql_ora/expected/ora_sysview.out` around lines 195 - 203, The test contains Oracle-style partitioning statements (CREATE TABLE t_partitioned (id INT, data VARCHAR2(100)) PARTITION BY RANGE (id) (...) and subsequent ALTER TABLE t_partitioned ADD CONSTRAINT pk_part PRIMARY KEY (id)) which are known to fail; remove these failing statements or rewrite them to use PostgreSQL-compatible syntax (e.g., CREATE TABLE t_partitioned (id INT, data VARCHAR(100)) PARTITION BY RANGE (id); CREATE TABLE t_partitioned_p1 PARTITION OF t_partitioned FOR VALUES FROM (MINVALUE) TO (100); CREATE TABLE t_partitioned_p2 PARTITION OF t_partitioned FOR VALUES FROM (100) TO (200); and then add the primary key via ALTER TABLE t_partitioned ADD CONSTRAINT pk_part PRIMARY KEY (id)). Ensure you update the expected output to reflect the successful PostgreSQL partitioning commands instead of the documented syntax errors.contrib/ivorysql_ora/src/sysview/sysview--1.0.sql (1)
1278-1282: Consider adding explicit type casts for Oracle compatibility.Other views in this file (e.g.,
DBA_PROCEDURES,ALL_PROCEDURES) consistently cast columns to Oracle-compatible types likeVARCHAR2(128). Theall_cons_columnsview lacks these casts, which may cause type inconsistencies.♻️ Proposed fix to add type casts
SELECT - pg_authid.rolname::VARCHAR2(128) AS owner, - con.conname::VARCHAR2(128) AS constraint_name, - cls.relname::VARCHAR2(128) AS table_name, - attr.attname::VARCHAR2(4000) AS column_name, - pos.pos::NUMBER AS position, + SYS.ORA_CASE_TRANS(pg_authid.rolname::VARCHAR2)::VARCHAR2(128) AS owner, + SYS.ORA_CASE_TRANS(con.conname::VARCHAR2)::VARCHAR2(128) AS constraint_name, + SYS.ORA_CASE_TRANS(cls.relname::VARCHAR2)::VARCHAR2(128) AS table_name, + SYS.ORA_CASE_TRANS(attr.attname::VARCHAR2)::VARCHAR2(4000) AS column_name, + pos.pos::NUMBER AS position,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contrib/ivorysql_ora/src/sysview/sysview--1.0.sql` around lines 1278 - 1282, The all_cons_columns view should use explicit Oracle-compatible casts consistent with other views: ensure pg_authid.rolname, con.conname and cls.relname are cast to VARCHAR2(128) for owner, constraint_name and table_name respectively; ensure attr.attname is cast to VARCHAR2(4000) for column_name and pos.pos is cast to NUMBER for position (update the SELECT expressions in the all_cons_columns view to use these ::VARCHAR2(...) / ::NUMBER casts so types align with DBA_PROCEDURES/ALL_PROCEDURES).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contrib/ivorysql_ora/expected/ora_sysview.out`:
- Around line 220-237: The expected output hardcodes the username "gsx2004"
causing failures for other users; update the expected result for the query that
uses WHERE owner = CURRENT_USER so it does not assume a specific owner: replace
the literal owner values in the output with a placeholder or pattern that
matches the actual CURRENT_USER at runtime (or generate the expected output
dynamically using CURRENT_USER), ensuring the rows for columns owner,
table_name, constraint_name, column_name, position still validate but the owner
field is not a fixed "gsx2004".
In `@contrib/ivorysql_ora/src/sysview/sysview_procedures.sql`:
- Around line 278-280: The view all_cons_columns defines owner using nsp.nspname
(namespace) which is incorrect for Oracle compatibility; change the owner
expression to use the table owner's role name (pg_authid.rolname) instead of
nsp.nspname — locate the SELECT that sets "AS owner" in the CREATE OR REPLACE
VIEW all_cons_columns and replace the nsp.nspname reference with the appropriate
join/lookup to pg_authid.rolname (as used in sysview--1.0.sql) so the owner
column returns the table owner's username.
- Line 278: The view is being created without the SYS schema qualifier and lacks
a public grant; change the CREATE OR REPLACE VIEW all_cons_columns AS to CREATE
OR REPLACE VIEW SYS.all_cons_columns AS (i.e. qualify the view name with SYS)
and add a corresponding GRANT SELECT ON SYS.all_cons_columns TO PUBLIC; after
the view definition so other users can access it; update any related references
or documentation if this was intentionally meant to be non‑privileged.
In `@contrib/ivorysql_ora/src/sysview/sysview--1.0.sql`:
- Around line 1276-1317: The repo has two conflicting ALL_CONS_COLUMNS views;
ensure Oracle semantics (OWNER is the constraint/table owner username and
partitioned tables included) by updating the other definition to match
SYS.all_cons_columns: use pg_authid.rolname as OWNER (not nsp.nspname), include
partitioned tables by using relkind IN ('r','v','m','p'), and keep the explicit
casts (e.g., ::VARCHAR2(128), ::NUMBER) and the same privilege checks
(has_schema_privilege, has_table_privilege) and joins (pg_constraint, pg_class,
pg_attribute, pg_type, unnest(conkey)) so both views return consistent results.
- Around line 1307-1308: The view currently uses CROSS JOIN LATERAL
unnest(con.conkey) which drops constraints with NULL/empty conkey
(expression-based CHECKs); update the all_cons_columns logic to handle those
cases by replacing the CROSS JOIN LATERAL unnest(con.conkey) with a LEFT JOIN
LATERAL that preserves rows when con.conkey IS NULL, and add handling for
contype = 'c' where conkey IS NULL to populate a representative row (e.g., set
attnum/column fields to NULL and include pg_get_constraintdef(con.oid) as an
expression column) or alternatively explicitly filter out expression-only CHECKs
from contype IN ('p','u','f','c') and document that behavior; reference
con.conkey, CROSS JOIN LATERAL unnest(con.conkey), contype IN ('p','u','f','c'),
and pg_get_constraintdef() to locate the change.
---
Nitpick comments:
In `@contrib/ivorysql_ora/expected/ora_sysview.out`:
- Around line 195-203: The test contains Oracle-style partitioning statements
(CREATE TABLE t_partitioned (id INT, data VARCHAR2(100)) PARTITION BY RANGE (id)
(...) and subsequent ALTER TABLE t_partitioned ADD CONSTRAINT pk_part PRIMARY
KEY (id)) which are known to fail; remove these failing statements or rewrite
them to use PostgreSQL-compatible syntax (e.g., CREATE TABLE t_partitioned (id
INT, data VARCHAR(100)) PARTITION BY RANGE (id); CREATE TABLE t_partitioned_p1
PARTITION OF t_partitioned FOR VALUES FROM (MINVALUE) TO (100); CREATE TABLE
t_partitioned_p2 PARTITION OF t_partitioned FOR VALUES FROM (100) TO (200); and
then add the primary key via ALTER TABLE t_partitioned ADD CONSTRAINT pk_part
PRIMARY KEY (id)). Ensure you update the expected output to reflect the
successful PostgreSQL partitioning commands instead of the documented syntax
errors.
In `@contrib/ivorysql_ora/sql/ora_sysview.sql`:
- Around line 134-137: Add verification for the data_type column in the
all_cons_columns test by extending the query coverage to select and assert
data_type mappings; specifically, add a test query against the all_cons_columns
view (same view name) that selects column_name and data_type for the test table
t_data_types (use WHERE owner = CURRENT_USER AND table_name = 't_data_types' and
ORDER BY column_name) so the test validates Oracle-compatible type mappings
alongside the existing owner/table_name/constraint_name/column_name/position
checks.
- Around line 109-113: The CREATE TABLE uses Oracle-style partition syntax which
is invalid in PostgreSQL; replace the block that creates t_partitioned and its
partitions and constraint by using PostgreSQL declarative partitioning: create
the parent table t_partitioned WITH PARTITION BY RANGE on column id (so the
partition key is id), then create separate child partitions (referencing
t_partitioned) using FOR VALUES FROM/TO (or FOR VALUES LESS THAN equivalents),
and finally add the primary key constraint pk_part on the parent table (ensuring
it includes the partition key id); update references to t_partitioned and
pk_part accordingly so the SQL is PostgreSQL-compatible.
In `@contrib/ivorysql_ora/src/sysview/sysview--1.0.sql`:
- Around line 1278-1282: The all_cons_columns view should use explicit
Oracle-compatible casts consistent with other views: ensure pg_authid.rolname,
con.conname and cls.relname are cast to VARCHAR2(128) for owner, constraint_name
and table_name respectively; ensure attr.attname is cast to VARCHAR2(4000) for
column_name and pos.pos is cast to NUMBER for position (update the SELECT
expressions in the all_cons_columns view to use these ::VARCHAR2(...) / ::NUMBER
casts so types align with DBA_PROCEDURES/ALL_PROCEDURES).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2c4a1722-e528-4d08-9e92-e3d026ad277f
📒 Files selected for processing (4)
contrib/ivorysql_ora/expected/ora_sysview.outcontrib/ivorysql_ora/sql/ora_sysview.sqlcontrib/ivorysql_ora/src/sysview/sysview--1.0.sqlcontrib/ivorysql_ora/src/sysview/sysview_procedures.sql
| CREATE OR REPLACE VIEW all_cons_columns AS | ||
| SELECT | ||
| nsp.nspname AS owner, |
There was a problem hiding this comment.
View uses schema name instead of table owner for owner column.
Oracle's ALL_CONS_COLUMNS.OWNER contains the table owner's username, not the schema name. Using nsp.nspname (schema/namespace name) will produce incorrect results compared to Oracle behavior.
The definition in sysview--1.0.sql correctly uses pg_authid.rolname (table owner). This definition should be aligned or removed to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contrib/ivorysql_ora/src/sysview/sysview_procedures.sql` around lines 278 -
280, The view all_cons_columns defines owner using nsp.nspname (namespace) which
is incorrect for Oracle compatibility; change the owner expression to use the
table owner's role name (pg_authid.rolname) instead of nsp.nspname — locate the
SELECT that sets "AS owner" in the CREATE OR REPLACE VIEW all_cons_columns and
replace the nsp.nspname reference with the appropriate join/lookup to
pg_authid.rolname (as used in sysview--1.0.sql) so the owner column returns the
table owner's username.
| OBJECT_ID | ||
| ; | ||
|
|
||
| CREATE OR REPLACE VIEW all_cons_columns AS |
There was a problem hiding this comment.
View missing SYS. schema prefix and GRANT statement.
This view is created without the SYS. schema prefix (unlike SYS.all_cons_columns in sysview--1.0.sql), which means:
- It will be created in the current schema rather than
SYS. - There's no
GRANT SELECT ON ... TO PUBLICstatement, so it won't be accessible to other users.
If this is intentional as a non-privileged alternative, consider adding documentation. Otherwise, align with the pattern used by other views.
♻️ Proposed fix to add SYS prefix and GRANT
-CREATE OR REPLACE VIEW all_cons_columns AS
+CREATE OR REPLACE VIEW SYS.all_cons_columns AS
SELECT
...
ORDER BY
owner, table_name, constraint_name, position;
+
+/* GRANT SELECT PRIVILEGE TO PUBLIC */
+GRANT SELECT ON SYS.all_cons_columns TO PUBLIC;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CREATE OR REPLACE VIEW all_cons_columns AS | |
| CREATE OR REPLACE VIEW SYS.all_cons_columns AS | |
| SELECT | |
| ... | |
| ORDER BY | |
| owner, table_name, constraint_name, position; | |
| /* GRANT SELECT PRIVILEGE TO PUBLIC */ | |
| GRANT SELECT ON SYS.all_cons_columns TO PUBLIC; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contrib/ivorysql_ora/src/sysview/sysview_procedures.sql` at line 278, The
view is being created without the SYS schema qualifier and lacks a public grant;
change the CREATE OR REPLACE VIEW all_cons_columns AS to CREATE OR REPLACE VIEW
SYS.all_cons_columns AS (i.e. qualify the view name with SYS) and add a
corresponding GRANT SELECT ON SYS.all_cons_columns TO PUBLIC; after the view
definition so other users can access it; update any related references or
documentation if this was intentionally meant to be non‑privileged.
| CREATE OR REPLACE VIEW SYS.all_cons_columns AS | ||
| SELECT | ||
| pg_authid.rolname::VARCHAR2(128) AS owner, -- 改为表属主 | ||
| con.conname::VARCHAR2(128) AS constraint_name, | ||
| cls.relname::VARCHAR2(128) AS table_name, | ||
| attr.attname::VARCHAR2(4000) AS column_name, | ||
| pos.pos::NUMBER AS position, | ||
| CASE | ||
| WHEN t.typname = 'varchar' THEN 'VARCHAR2' | ||
| WHEN t.typname = 'bpchar' THEN 'CHAR' | ||
| WHEN t.typname = 'numeric' THEN 'NUMBER' | ||
| WHEN t.typname = 'int2' THEN 'NUMBER' | ||
| WHEN t.typname = 'int4' THEN 'NUMBER' | ||
| WHEN t.typname = 'int8' THEN 'NUMBER' | ||
| WHEN t.typname = 'float4' THEN 'FLOAT' | ||
| WHEN t.typname = 'float8' THEN 'FLOAT' | ||
| WHEN t.typname = 'text' THEN 'CLOB' | ||
| WHEN t.typname = 'date' THEN 'DATE' | ||
| WHEN t.typname = 'timestamp' THEN 'TIMESTAMP' | ||
| WHEN t.typname = 'timestamptz' THEN 'TIMESTAMP WITH TIME ZONE' | ||
| ELSE UPPER(t.typname) | ||
| END AS data_type | ||
| FROM | ||
| pg_catalog.pg_constraint con | ||
| JOIN pg_catalog.pg_class cls ON con.conrelid = cls.oid | ||
| JOIN pg_catalog.pg_authid ON cls.relowner = pg_authid.oid -- 关联属主 | ||
| JOIN pg_catalog.pg_namespace nsp ON cls.relnamespace = nsp.oid | ||
| CROSS JOIN LATERAL unnest(con.conkey) WITH ORDINALITY AS pos(attnum, pos) | ||
| JOIN pg_catalog.pg_attribute attr ON attr.attrelid = cls.oid | ||
| AND attr.attnum = pos.attnum | ||
| JOIN pg_catalog.pg_type t ON attr.atttypid = t.oid | ||
| WHERE | ||
| con.contype IN ('p', 'u', 'f', 'c') | ||
| AND nsp.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast') | ||
| AND cls.relkind IN ('r', 'v', 'm', 'p') | ||
| AND attr.attnum > 0 | ||
| AND NOT attr.attisdropped | ||
| AND has_schema_privilege(nsp.oid, 'USAGE') | ||
| AND has_table_privilege(cls.oid, 'SELECT') | ||
| ORDER BY | ||
| owner, table_name, constraint_name, position; | ||
| GRANT SELECT ON SYS.all_cons_columns TO PUBLIC; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all definitions of all_cons_columns view
echo "=== Searching for all_cons_columns definitions ==="
rg -n -i "CREATE.*VIEW.*all_cons_columns" --type sql
echo ""
echo "=== Check owner column definition in each file ==="
rg -n -A2 "AS owner" --type sql -g "*sysview*"Repository: IvorySQL/IvorySQL
Length of output: 1080
🏁 Script executed:
#!/bin/bash
# Check relkind filter in both views
echo "=== relkind filter in sysview--1.0.sql ==="
sed -n '1276,1317p' contrib/ivorysql_ora/src/sysview/sysview--1.0.sql | grep -i "relkind"
echo ""
echo "=== relkind filter in sysview_procedures.sql ==="
sed -n '278,350p' contrib/ivorysql_ora/src/sysview/sysview_procedures.sql | grep -i "relkind"
echo ""
echo "=== Full view definition in sysview_procedures.sql ==="
# Get line count of the file first
total_lines=$(wc -l < contrib/ivorysql_ora/src/sysview/sysview_procedures.sql)
echo "File has $total_lines lines"
# Show the view definition
sed -n '278,320p' contrib/ivorysql_ora/src/sysview/sysview_procedures.sqlRepository: IvorySQL/IvorySQL
Length of output: 2224
Conflicting view definitions cause inconsistent results.
Two versions of all_cons_columns exist with significant semantic differences:
-
SYS.all_cons_columns(sysview--1.0.sql, lines 1276-1317):- Owner column:
pg_authid.rolname(table owner/role name) - Includes partitioned tables:
relkind IN ('r', 'v', 'm', 'p') - Uses explicit type casts (
::VARCHAR2(128),::NUMBER)
- Owner column:
-
all_cons_columns(sysview_procedures.sql, lines 278-320):- Owner column:
nsp.nspname(schema name) - Excludes partitioned tables:
relkind IN ('r', 'v', 'm') - No type casts
- Owner column:
Oracle's ALL_CONS_COLUMNS.OWNER represents the constraint/table owner (username), not the schema name. The current inconsistency means queries will return different result sets depending on which view is used, and the sysview_procedures.sql version violates Oracle compatibility semantics.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contrib/ivorysql_ora/src/sysview/sysview--1.0.sql` around lines 1276 - 1317,
The repo has two conflicting ALL_CONS_COLUMNS views; ensure Oracle semantics
(OWNER is the constraint/table owner username and partitioned tables included)
by updating the other definition to match SYS.all_cons_columns: use
pg_authid.rolname as OWNER (not nsp.nspname), include partitioned tables by
using relkind IN ('r','v','m','p'), and keep the explicit casts (e.g.,
::VARCHAR2(128), ::NUMBER) and the same privilege checks (has_schema_privilege,
has_table_privilege) and joins (pg_constraint, pg_class, pg_attribute, pg_type,
unnest(conkey)) so both views return consistent results.
| WHERE | ||
| con.contype IN ('p', 'u', 'f', 'c') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify CHECK constraint behavior with expression-based constraints
# Check if there are any CHECK constraints with NULL or empty conkey in PostgreSQL catalogs
cat << 'EOF'
-- Test query to find CHECK constraints without column references:
SELECT con.conname, con.conkey
FROM pg_constraint con
WHERE con.contype = 'c' AND (con.conkey IS NULL OR array_length(con.conkey, 1) IS NULL);
EOFRepository: IvorySQL/IvorySQL
Length of output: 270
🏁 Script executed:
# First, find and examine the file
fd sysview--1.0.sql -t fRepository: IvorySQL/IvorySQL
Length of output: 110
🏁 Script executed:
# Read the relevant section with more context
if [ -f "contrib/ivorysql_ora/src/sysview/sysview--1.0.sql" ]; then
sed -n '1290,1320p' "contrib/ivorysql_ora/src/sysview/sysview--1.0.sql"
fiRepository: IvorySQL/IvorySQL
Length of output: 1419
🏁 Script executed:
# Search for unnest(con.conkey) usage in the file to understand the full context
rg "unnest.*con\.conkey" contrib/ivorysql_ora/src/sysview/ -A 5 -B 5Repository: IvorySQL/IvorySQL
Length of output: 2340
🌐 Web query:
PostgreSQL pg_constraint conkey NULL expression-based CHECK constraints
💡 Result:
In PostgreSQL's pg_constraint system catalog, the conkey field (int2[] array referencing pg_attribute.attnum) lists the constrained column numbers for table constraints that directly reference columns, such as primary key, unique, foreign key, and exclusion constraints on simple column references. For CHECK constraints (contype='c'), including expression-based ones, conkey is typically an empty array {} because CHECK constraints use arbitrary Boolean expressions stored in conbin (pg_node_tree), not limited to specific columns. The official documentation does not explicitly state conkey is NULL for CHECK constraints; as an array type, it defaults to empty rather than SQL NULL. Column-defined CHECK constraints (e.g., column CHECK (expr)) are treated equivalently to table constraints, so conkey remains empty or may contain the column number in some implementations, but the expression is always in conbin. Use pg_get_constraintdef(oid) to retrieve the full definition. To identify CHECK constraints, query WHERE contype = 'c'; conkey will not reliably indicate columns for them. Not-null constraints (contype='n') are in pg_attribute.attnotnull, not pg_constraint.
Citations:
- 1: https://www.postgresql.org/docs/current/catalog-pg-constraint.html
- 2: https://stackoverflow.com/questions/60535831/how-to-get-column-name-from-pg-constraint
- 3: https://www.postgresql.org/docs/current/ddl-constraints.html
🌐 Web query:
IvorySQL CHECK constraint expression-only definition conkey NULL
💡 Result:
In IvorySQL, which is a PostgreSQL fork with Oracle compatibility features, CHECK constraint definitions that are expression-only (i.e., table-level CHECK constraints not tied to specific columns, or column constraints treated as table constraints) have conkey set to NULL in the pg_constraint system catalog. This matches PostgreSQL behavior, where conkey (an int2[] array referencing pg_attribute.attnum) lists constrained columns for table constraints like PK/Unique/FK, but for CHECK constraints (contype='c'), it is typically an empty array or NULL since the constraint is defined by the expression in conbin (pg_node_tree), not specific columns. Official PostgreSQL documentation states: "If a table constraint (including foreign keys, but not constraint triggers), list of the constrained columns" — CHECK constraints do not populate conkey with column numbers when expression-only. IvorySQL documentation references and system catalogs align with PostgreSQL, with no deviations noted for pg_constraint.
Citations:
- 1: https://www.postgresql.org/docs/current/catalog-pg-constraint.html
- 2: https://docs.ivorysql.org/en/ivorysql-doc/v5.3/1
- 3: https://docs.ivorysql.org/en/ivorysql-doc/v5.3/4.3
🏁 Script executed:
# Search for any comments or documentation in the codebase about this behavior
rg "CHECK.*expression" contrib/ivorysql_ora/src/sysview/ -iRepository: IvorySQL/IvorySQL
Length of output: 43
🏁 Script executed:
# Check if there are any related issues or tests
fd . contrib/ivorysql_ora -name "*.sql" -type f | xargs rg "expression-based|expression.*CHECK" -i 2>/dev/null | head -20Repository: IvorySQL/IvorySQL
Length of output: 2354
🏁 Script executed:
# Search for any conditional handling of conkey in other views
rg "conkey.*NULL\|conkey.*empty" contrib/ivorysql_ora/ -iRepository: IvorySQL/IvorySQL
Length of output: 43
🏁 Script executed:
# Let me check if there's any special handling or workarounds in the codebase
rg "conkey.*IS NULL\|conkey.*= '{}" contrib/ivorysql_ora/src/sysview/Repository: IvorySQL/IvorySQL
Length of output: 202
🏁 Script executed:
# Check if there are any issues or TODOs related to CHECK constraints
rg "TODO\|FIXME\|BUG\|CHECK.*constraint" contrib/ivorysql_ora/src/sysview/sysview--1.0.sql -iRepository: IvorySQL/IvorySQL
Length of output: 43
🏁 Script executed:
# Look at the full view definition to see if there's any special handling we missed
wc -l contrib/ivorysql_ora/src/sysview/sysview--1.0.sql
sed -n '1280,1330p' contrib/ivorysql_ora/src/sysview/sysview--1.0.sqlRepository: IvorySQL/IvorySQL
Length of output: 1971
🏁 Script executed:
# Let me check the view name and purpose to understand if excluding expression-based CHECK constraints is intentional
rg "CREATE.*VIEW.*all_cons_columns\|CREATE OR REPLACE VIEW" contrib/ivorysql_ora/src/sysview/sysview--1.0.sql -B 5 -A 50 | head -60Repository: IvorySQL/IvorySQL
Length of output: 43
🏁 Script executed:
# Search for the view definition more broadly
rg "all_cons_columns" contrib/ivorysql_ora/src/sysview/sysview--1.0.sql -B 10Repository: IvorySQL/IvorySQL
Length of output: 868
🏁 Script executed:
# Look at the structure of the view - check if this is Oracle-compatible all_cons_columns
sed -n '1270,1290p' contrib/ivorysql_ora/src/sysview/sysview--1.0.sqlRepository: IvorySQL/IvorySQL
Length of output: 998
🏁 Script executed:
# Check Oracle documentation reference or comments in the file about all_cons_columns
rg "all_cons_columns\|constraint.*column" contrib/ivorysql_ora/src/sysview/ -B 2 -A 2 | head -40Repository: IvorySQL/IvorySQL
Length of output: 43
🏁 Script executed:
# Let me check if there's any Oracle all_cons_columns documentation or comments about this behavior
rg "all_cons_columns\|CHECK.*expression\|expression.*CHECK" contrib/ivorysql_ora/ -i -B 3 -A 3 | head -50Repository: IvorySQL/IvorySQL
Length of output: 43
🏁 Script executed:
# Check if there are any test files that validate this view's behavior
fd "test.*cons.*columns\|all_cons_columns" contrib/ivorysql_ora/ -type fRepository: IvorySQL/IvorySQL
Length of output: 231
🏁 Script executed:
# Let me check Oracle's all_cons_columns documentation reference or any comments explaining the view purpose
sed -n '1270,1280p' contrib/ivorysql_ora/src/sysview/sysview--1.0.sqlRepository: IvorySQL/IvorySQL
Length of output: 515
🏁 Script executed:
# Search for any related Oracle compatibility notes
fd "*.md" contrib/ivorysql_ora/ -type fRepository: IvorySQL/IvorySQL
Length of output: 231
Handle expression-based CHECK constraints in all_cons_columns view.
The CROSS JOIN LATERAL unnest(con.conkey) silently excludes expression-based CHECK constraints because conkey is NULL or empty for constraints that don't reference specific columns. This causes incomplete results—the view claims to show all constraint columns but misses CHECK constraints like CHECK (col1 + col2 > 10). Consider adding separate handling for expression-based CHECK constraints using pg_get_constraintdef() or filtering them out explicitly from the contype IN ('p', 'u', 'f', 'c') clause with clear documentation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contrib/ivorysql_ora/src/sysview/sysview--1.0.sql` around lines 1307 - 1308,
The view currently uses CROSS JOIN LATERAL unnest(con.conkey) which drops
constraints with NULL/empty conkey (expression-based CHECKs); update the
all_cons_columns logic to handle those cases by replacing the CROSS JOIN LATERAL
unnest(con.conkey) with a LEFT JOIN LATERAL that preserves rows when con.conkey
IS NULL, and add handling for contype = 'c' where conkey IS NULL to populate a
representative row (e.g., set attnum/column fields to NULL and include
pg_get_constraintdef(con.oid) as an expression column) or alternatively
explicitly filter out expression-only CHECKs from contype IN ('p','u','f','c')
and document that behavior; reference con.conkey, CROSS JOIN LATERAL
unnest(con.conkey), contype IN ('p','u','f','c'), and pg_get_constraintdef() to
locate the change.
|
review is in progress, thanks. |
|
|
||
| CREATE OR REPLACE VIEW SYS.all_cons_columns AS | ||
| SELECT | ||
| pg_authid.rolname::VARCHAR2(128) AS owner, -- 改为表属主 |
There was a problem hiding this comment.
please rewrite the comment in English.
| FROM | ||
| pg_catalog.pg_constraint con | ||
| JOIN pg_catalog.pg_class cls ON con.conrelid = cls.oid | ||
| JOIN pg_catalog.pg_authid ON cls.relowner = pg_authid.oid -- 关联属主 |
There was a problem hiding this comment.
please rewrite teh comment in English.
| WHEN t.typname = 'timestamp' THEN 'TIMESTAMP' | ||
| WHEN t.typname = 'timestamptz' THEN 'TIMESTAMP WITH TIME ZONE' | ||
| ELSE UPPER(t.typname) | ||
| END AS data_type |
There was a problem hiding this comment.
Why do you output data_type here?
Oracle would not do so.
|
For issue 1010: |
|
|
||
| CREATE OR REPLACE VIEW SYS.all_cons_columns AS | ||
| SELECT | ||
| pg_authid.rolname::VARCHAR2(128) AS owner, -- table owner |
There was a problem hiding this comment.
[Same comment as scmysxb-git gave to PR1201]:
Oracle query results are uppercase by default, whereas IvorySQL uses lowercase by default. To maintain consistency with Oracle, IvorySQL provides the case-conversion function ORA_CASE_TRANS. It is recommended to use this function for automatic case conversion to match Oracle's behavior.
add all_cons_columns view
Summary by CodeRabbit
New Features
Tests