Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions contrib/ivorysql_ora/expected/ora_sysview.out
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,22 @@ ORDER BY NAME;
listen_addresses | 2 | | localhost | FALSE | FALSE | FALSE | Sets the host name or IP address(es) to listen to.
(5 rows)

-- Test ALL_TAB_COLUMNS
DROP TABLE IF EXISTS test_user_cons;
NOTICE: table "test_user_cons" does not exist, skipping
CREATE TABLE test_user_cons (
id int primary key,
name varchar(50) not null
);
SELECT OWNER, TABLE_NAME, COLUMN_NAME, COLUMN_ID,
DATA_TYPE, DATA_LENGTH, DATA_PRECISION, DATA_SCALE,
NULLABLE
FROM ALL_TAB_COLUMNS
WHERE TABLE_NAME = 'test_user_cons';
owner | table_name | column_name | column_id | data_type | data_length | data_precision | data_scale | nullable
--------+----------------+-------------+-----------+----------------+-------------+----------------+------------+----------
public | test_user_cons | id | 1 | int4 | 4 | | | N
public | test_user_cons | name | 2 | oravarcharbyte | -1 | | | N
(2 rows)

DROP TABLE IF EXISTS test_user_cons;
16 changes: 16 additions & 0 deletions contrib/ivorysql_ora/sql/ora_sysview.sql
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,19 @@ SELECT *
FROM SYS.V$PARAMETER
WHERE NAME IN ('listen_addresses','application_name','archive_command','archive_mode','block_size')
ORDER BY NAME;

-- Test ALL_TAB_COLUMNS
DROP TABLE IF EXISTS test_user_cons;

CREATE TABLE test_user_cons (
id int primary key,
name varchar(50) not null
);
Comment on lines +97 to +100
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Expand this test to cover untested ALL_TAB_COLUMNS branches.

Current case validates only int4 and varchar with NOT NULL. It misses numeric precision/scale and NULLABLE = 'Y' paths implemented by the view.

Suggested expansion
 CREATE TABLE test_user_cons (
     id int primary key,
-    name varchar(50) not null
+    name varchar(50) not null,
+    amount numeric(10,2),
+    code char(3)
 );
 SELECT OWNER, TABLE_NAME, COLUMN_NAME, COLUMN_ID,
        DATA_TYPE, DATA_LENGTH, DATA_PRECISION, DATA_SCALE,
        NULLABLE
 FROM ALL_TAB_COLUMNS
 WHERE TABLE_NAME = 'test_user_cons'
   AND OWNER = current_schema();
+ORDER BY COLUMN_ID;
As per coding guidelines, `**/sql/*.sql`: Test SQL files. Ensure comprehensive coverage of features.

Also applies to: 102-104

🤖 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 97 - 100, Extend the
test table definition used by the ALL_TAB_COLUMNS coverage by adding columns
that exercise numeric precision/scale and nullable cases: keep the existing id
(int4) and name (varchar) columns, then add a numeric/decimal column with
explicit precision and scale (e.g., NUMERIC(p,s)) to trigger precision/scale
handling, and add at least one column declared NULLABLE (no NOT NULL) to cover
the NULLABLE = 'Y' path; update any assertions in the test to expect the
precision/scale values and nullable flag for the new columns so ALL_TAB_COLUMNS
branches are exercised (refer to test_user_cons and the view logic that reads
column_precision/column_scale and nullable status).


SELECT OWNER, TABLE_NAME, COLUMN_NAME, COLUMN_ID,
DATA_TYPE, DATA_LENGTH, DATA_PRECISION, DATA_SCALE,
NULLABLE
FROM ALL_TAB_COLUMNS
WHERE TABLE_NAME = 'test_user_cons';
Comment on lines +102 to +106
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Scope the assertion to schema to avoid cross-schema flakiness.

At Line 106, filtering only by TABLE_NAME can return extra rows if another schema has the same table name.

Suggested fix
 SELECT OWNER, TABLE_NAME, COLUMN_NAME, COLUMN_ID,
        DATA_TYPE, DATA_LENGTH, DATA_PRECISION, DATA_SCALE,
        NULLABLE
 FROM ALL_TAB_COLUMNS
-WHERE TABLE_NAME = 'test_user_cons';
+WHERE TABLE_NAME = 'test_user_cons'
+  AND OWNER = current_schema();
📝 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.

Suggested change
SELECT OWNER, TABLE_NAME, COLUMN_NAME, COLUMN_ID,
DATA_TYPE, DATA_LENGTH, DATA_PRECISION, DATA_SCALE,
NULLABLE
FROM ALL_TAB_COLUMNS
WHERE TABLE_NAME = 'test_user_cons';
SELECT OWNER, TABLE_NAME, COLUMN_NAME, COLUMN_ID,
DATA_TYPE, DATA_LENGTH, DATA_PRECISION, DATA_SCALE,
NULLABLE
FROM ALL_TAB_COLUMNS
WHERE TABLE_NAME = 'test_user_cons'
AND OWNER = current_schema();
🤖 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 102 - 106, The SELECT
against ALL_TAB_COLUMNS is only filtering by TABLE_NAME ('test_user_cons') which
can return rows from other schemas; update the WHERE clause in the query that
references ALL_TAB_COLUMNS to also restrict OWNER (e.g., OWNER = current_schema
or the expected schema name) so the assertion is scoped to the intended schema
and avoids cross-schema flakiness.


DROP TABLE IF EXISTS test_user_cons;
36 changes: 36 additions & 0 deletions contrib/ivorysql_ora/src/sysview/sysview--1.0.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1273,3 +1273,39 @@ SELECT
SHORT_DESC::VARCHAR2(255) AS DESCRIPTION
FROM PG_SETTINGS;

/*
* ALL_TAB_COLUMNS - Oracle-compatible columns view
*/
CREATE OR REPLACE VIEW ALL_TAB_COLUMNS AS
SELECT
n.nspname AS owner,
c.relname AS table_name,
a.attname AS column_name,
a.attnum AS column_id,
t.typname AS data_type,
CASE
WHEN t.typname IN ('varchar', 'char', 'bpchar') THEN (a.atttypmod - 4)
WHEN t.typname = 'numeric' THEN ((a.atttypmod - 4) >> 16)
ELSE a.attlen
END AS data_length,
CASE
WHEN t.typname = 'numeric' THEN ((a.atttypmod - 4) >> 16)
ELSE NULL
END AS data_precision,
CASE
WHEN t.typname = 'numeric' THEN ((a.atttypmod - 4) & 65535)
ELSE NULL
END AS data_scale,
CASE WHEN a.attnotnull THEN 'N' ELSE 'Y' END AS nullable
FROM pg_class c
JOIN pg_attribute a ON a.attrelid = c.oid
JOIN pg_type t ON a.atttypid = t.oid
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE a.attnum > 0
AND NOT a.attisdropped
AND c.relkind IN ('r', 'v')
AND n.nspname NOT IN ('pg_catalog', 'information_schema')
ORDER BY n.nspname, c.relname, a.attnum;
Comment on lines +1279 to +1308
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Missing SYS. schema prefix for consistency with other views.

All other views in this file are created under the SYS schema (e.g., SYS.ALL_PROCEDURES, SYS.ALL_SOURCE, SYS.ALL_ARGUMENTS), but ALL_TAB_COLUMNS is created without the schema prefix. Additionally:

  1. Missing SYS.ORA_CASE_TRANS() calls for owner, table_name, and column_name columns to match Oracle identifier casing behavior
  2. Missing privilege checks like HAS_SCHEMA_PRIVILEGE() that other ALL_* views include
Proposed fix for schema and consistency
-CREATE OR REPLACE VIEW ALL_TAB_COLUMNS AS
+CREATE OR REPLACE VIEW SYS.ALL_TAB_COLUMNS AS
 SELECT
-    n.nspname AS owner,
-    c.relname AS table_name,
-    a.attname AS column_name,
+    SYS.ORA_CASE_TRANS(n.nspname::VARCHAR2)::VARCHAR2(128) AS owner,
+    SYS.ORA_CASE_TRANS(c.relname::VARCHAR2)::VARCHAR2(128) AS table_name,
+    SYS.ORA_CASE_TRANS(a.attname::VARCHAR2)::VARCHAR2(128) AS column_name,
     a.attnum AS column_id,
     t.typname AS data_type,
     ...
 WHERE a.attnum > 0
   AND NOT a.attisdropped
   AND c.relkind IN ('r', 'v')
   AND n.nspname NOT IN ('pg_catalog', 'information_schema')
+  AND HAS_SCHEMA_PRIVILEGE(n.oid, 'USAGE')
 ORDER BY n.nspname, c.relname, a.attnum;
🤖 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 1279 - 1308,
Rename the view to be created as SYS.ALL_TAB_COLUMNS (not unqualified
ALL_TAB_COLUMNS); wrap owner, table_name and column_name expressions with
SYS.ORA_CASE_TRANS() to match Oracle identifier casing behavior (replace
n.nspname, c.relname, a.attname with SYS.ORA_CASE_TRANS(n.nspname) etc. and
adjust the ORDER BY accordingly); and add the same privilege filtering used by
other ALL_* views (e.g., include the schema/table visibility check using
HAS_SCHEMA_PRIVILEGE/appropriate HAS_* privilege calls on n.nspname/c.oid so
only accessible columns are returned). Ensure references to
pg_class/pg_attribute/pg_type/pg_namespace remain the same and keep the relkind
and att filters.


GRANT SELECT ON ALL_TAB_COLUMNS TO PUBLIC;
COMMENT ON VIEW ALL_TAB_COLUMNS IS 'Oracle-style ALL_TAB_COLUMNS view';
Comment on lines +1310 to +1311
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Update GRANT to reference the correct schema.

If the view is moved to SYS schema as suggested above, the GRANT and COMMENT statements should also reference SYS.ALL_TAB_COLUMNS.

Proposed fix
-GRANT SELECT ON ALL_TAB_COLUMNS TO PUBLIC;
-COMMENT ON VIEW ALL_TAB_COLUMNS IS 'Oracle-style ALL_TAB_COLUMNS view';
+GRANT SELECT ON SYS.ALL_TAB_COLUMNS TO PUBLIC;
+COMMENT ON VIEW SYS.ALL_TAB_COLUMNS IS 'Oracle-style ALL_TAB_COLUMNS view';
📝 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.

Suggested change
GRANT SELECT ON ALL_TAB_COLUMNS TO PUBLIC;
COMMENT ON VIEW ALL_TAB_COLUMNS IS 'Oracle-style ALL_TAB_COLUMNS view';
GRANT SELECT ON SYS.ALL_TAB_COLUMNS TO PUBLIC;
COMMENT ON VIEW SYS.ALL_TAB_COLUMNS IS 'Oracle-style ALL_TAB_COLUMNS view';
🤖 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 1310 - 1311,
Update the GRANT and COMMENT statements to reference the view with its
schema-qualified name; change the unqualified references of ALL_TAB_COLUMNS to
SYS.ALL_TAB_COLUMNS in the GRANT SELECT ON and COMMENT ON VIEW lines so they
operate on the SYS schema view (i.e., update the statements that currently read
"GRANT SELECT ON ALL_TAB_COLUMNS TO PUBLIC;" and "COMMENT ON VIEW
ALL_TAB_COLUMNS ..." to use SYS.ALL_TAB_COLUMNS).

Loading