Skip to content
Merged
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
4 changes: 3 additions & 1 deletion contrib/ivorysql_ora/preload_ora_misc.sql
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ insert into dual values('X');
GRANT SELECT ON dual TO PUBLIC;

--
-- ROWID type
-- Oracle-compatible ROWID and UROWID types
-- ROWID: Composite type containing row object ID and row number
-- UROWID: Universal ROWID, compatible with ROWID
--
CREATE TYPE sys.rowid AS(rowoid OID, rowno bigint);
CREATE TYPE sys.urowid AS(rowoid OID, rowno bigint);
Expand Down
1 change: 1 addition & 0 deletions contrib/tablefunc/expected/ivy_tablefunc.out
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ SELECT avg(normal_rand)::int, count(*) FROM normal_rand(-1, 250, 0.2);
ERROR: number of rows cannot be negative
--
-- crosstab()
-- Use 'orarowid' instead of 'rowid' to avoid conflict with system column
--
CREATE TABLE ct(id int, rowclass text, orarowid text, attribute text, val text);
\copy ct from 'data/ct.data'
Expand Down
1 change: 1 addition & 0 deletions contrib/tablefunc/sql/ivy_tablefunc.sql
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ SELECT avg(normal_rand)::int, count(*) FROM normal_rand(-1, 250, 0.2);

--
-- crosstab()
-- Use 'orarowid' instead of 'rowid' to avoid conflict with system column
--
CREATE TABLE ct(id int, rowclass text, orarowid text, attribute text, val text);
\copy ct from 'data/ct.data'
Expand Down
4 changes: 3 additions & 1 deletion src/backend/catalog/heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ static const FormData_pg_attribute a6 = {
};

/*
* Compatible Oracle ROWID pseudo column.
* Oracle-compatible ROWID pseudo-column.
* This system column provides a unique identifier for each row,
* compatible with Oracle's ROWID functionality.
*/
static const FormData_pg_attribute a7 = {
.attname = {"rowid"},
Expand Down
111 changes: 60 additions & 51 deletions src/backend/parser/parse_utilcmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
#include "utils/ruleutils.h"
#include "utils/syscache.h"
#include "utils/typcache.h"
#include <math.h>
#include <math.h>
#include "utils/guc.h"
#include "utils/ora_compatible.h"

Expand Down Expand Up @@ -280,10 +280,12 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
if (cxt.inhRelations)
{
ListCell *inher;

foreach(inher, cxt.inhRelations)
{
RangeVar *inh = lfirst_node(RangeVar, inher);
Relation prel;

prel = table_openrv(inh, AccessShareLock);
cxt.hasrowid = cxt.hasrowid || prel->rd_rel->relhasrowid;
table_close(prel, NoLock);
Expand All @@ -302,7 +304,8 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
{
case T_ColumnDef:
{
ColumnDef *col = (ColumnDef *) element;
ColumnDef *col = (ColumnDef *) element;

if (compatible_db == ORA_PARSER && strcmp(col->colname, "rowid") == 0)
elog(ERROR, "column name \"%s\" conflicts with a system column name", col->colname);
else
Expand Down Expand Up @@ -331,13 +334,14 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
*/
foreach(elements, cxt.columns)
{
ColumnDef *element = lfirst(elements);
if(element->identity == ATTRIBUTE_IDENTITY_DEFAULT_ON_NULL ||
element->identity == ATTRIBUTE_ORA_IDENTITY_ALWAYS ||
element->identity == ATTRIBUTE_ORA_IDENTITY_BY_DEFAULT)
ColumnDef *element = lfirst(elements);

if (element->identity == ATTRIBUTE_IDENTITY_DEFAULT_ON_NULL ||
element->identity == ATTRIBUTE_ORA_IDENTITY_ALWAYS ||
element->identity == ATTRIBUTE_ORA_IDENTITY_BY_DEFAULT)
ora_identity_cnt++;
}
if(ora_identity_cnt > 1)
if (ora_identity_cnt > 1)
elog(ERROR, "table can have only one identity column");

if (like_found && cxt.hasrowid)
Expand All @@ -348,7 +352,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)

if (cxt.hasrowid)
{
Oid snamespaceid;
Oid snamespaceid;
char *snamespace;
char *sname;

Expand All @@ -368,47 +372,48 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
false);

/*
* Build a CREATE SEQUENCE command to create the sequence object,
* and add it to the list of things to be done before this CREATE/ALTER TABLE
* Build a CREATE SEQUENCE command to create the sequence object, and
* add it to the list of things to be done before this CREATE/ALTER
* TABLE
*/
seqstmt = makeNode(CreateSeqStmt);
seqstmt->with_rowid = true;
seqstmt->sequence = makeRangeVar(snamespace, sname, -1);
seqstmt->options = lcons(makeDefElem("as",
(Node *) makeTypeNameFromOid(INT8OID, -1),
-1),
seqstmt->options);
(Node *) makeTypeNameFromOid(INT8OID, -1),
-1),
seqstmt->options);
if (rowid_seq_cache > 1)
{
seqstmt->options = lcons(makeDefElem("cache",
(Node *) makeInteger(rowid_seq_cache),
-1),
seqstmt->options);
(Node *) makeInteger(rowid_seq_cache),
-1),
seqstmt->options);
}
else
seqstmt->options = lcons(makeDefElem("nocache",
NULL,
-1),
seqstmt->options);
NULL,
-1),
seqstmt->options);

if (cxt.rel)
seqstmt->ownerId = cxt.rel->rd_rel->relowner;

cxt.blist = lappend(cxt.blist, seqstmt);

/*
* Build an ALTER SEQUENCE ... OWNED BY command to mark the sequence as owned
* by this table.
* Build an ALTER SEQUENCE ... OWNED BY command to mark the sequence
* as owned by this table.
*/
altseqstmt = makeNode(AlterSeqStmt);
altseqstmt->sequence = makeRangeVar(snamespace, sname, -1);

relnamelist = list_make3(makeString(snamespace),
makeString(cxt.relation->relname),
makeString(sname));
makeString(cxt.relation->relname),
makeString(sname));

altseqstmt->options = list_make1(makeDefElem("owned_by",
(Node *) relnamelist, -1));
(Node *) relnamelist, -1));
cxt.alist = lappend(cxt.alist, altseqstmt);
Comment on lines 411 to 417
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 | 🔴 Critical

Fix OWNED BY target column in rowid sequence setup.

ALTER SEQUENCE schema.table_rowid_seq OWNED BY schema.table.table_rowid_seq will run because the third identifier comes from sname, i.e., the sequence name. The table has no column of that name, so the ALTER will error out and the table creation fails. The third element must be the ROWID column name (e.g., "rowid"), not the sequence name. Please adjust the relnamelist accordingly.

Apply this diff to correct the relnamelist:

-       relnamelist = list_make3(makeString(snamespace),
-                                 makeString(cxt->relation->relname),
-                                 makeString(sname));
+       relnamelist = list_make3(makeString(snamespace),
+                                 makeString(cxt->relation->relname),
+                                 makeString("rowid"));
📝 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
relnamelist = list_make3(makeString(snamespace),
makeString(cxt.relation->relname),
makeString(sname));
makeString(cxt.relation->relname),
makeString(sname));
altseqstmt->options = list_make1(makeDefElem("owned_by",
(Node *) relnamelist, -1));
(Node *) relnamelist, -1));
cxt.alist = lappend(cxt.alist, altseqstmt);
relnamelist = list_make3(makeString(snamespace),
makeString(cxt.relation->relname),
makeString("rowid"));
altseqstmt->options = list_make1(makeDefElem("owned_by",
(Node *) relnamelist, -1));
cxt.alist = lappend(cxt.alist, altseqstmt);

}

Expand Down Expand Up @@ -971,11 +976,14 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
ctype = typenameType(cxt->pstate, column->typeName, &typmod);
typeOid = ((Form_pg_type) GETSTRUCT(ctype))->oid;

/* Convert compatible identity smallint/int type column to bigint type */
/*
* Convert compatible identity smallint/int type column to
* bigint type
*/
if ((constraint->generated_when == ATTRIBUTE_IDENTITY_DEFAULT_ON_NULL
|| constraint->generated_when == ATTRIBUTE_ORA_IDENTITY_ALWAYS
|| constraint->generated_when == ATTRIBUTE_ORA_IDENTITY_BY_DEFAULT)
&& (typeOid == INT4OID || typeOid == INT2OID))
|| constraint->generated_when == ATTRIBUTE_ORA_IDENTITY_ALWAYS
|| constraint->generated_when == ATTRIBUTE_ORA_IDENTITY_BY_DEFAULT)
&& (typeOid == INT4OID || typeOid == INT2OID))
{
column->typeName = makeTypeName("int8");
typeOid = INT8OID;
Expand Down Expand Up @@ -1296,7 +1304,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
if (relation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
{
aclresult = object_aclcheck(TypeRelationId, relation->rd_rel->reltype, GetUserId(),
ACL_USAGE);
ACL_USAGE);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, OBJECT_TYPE,
RelationGetRelationName(relation));
Expand Down Expand Up @@ -2689,7 +2697,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
* mentioned above.
*/
Datum attoptions =
get_attoptions(RelationGetRelid(index_rel), i + 1);
get_attoptions(RelationGetRelid(index_rel), i + 1);

defopclass = GetDefaultOpClass(attform->atttypid,
index_rel->rd_rel->relam);
Expand Down Expand Up @@ -3764,12 +3772,12 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,

case AT_AddRowidsRecurse:
{
Oid snamespaceid;
char *snamespace;
char *sname;
List *relnamelist;
CreateSeqStmt *seqstmt;
AlterSeqStmt *altseqstmt;
Oid snamespaceid;
char *snamespace;
char *sname;
List *relnamelist;
CreateSeqStmt *seqstmt;
AlterSeqStmt *altseqstmt;

if (cmd->is_rowid)
{
Expand All @@ -3789,45 +3797,46 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
false);

/*
* Build a CREATE SEQUENCE command to create the sequence object,
* and add it to the list of things to be done before this CREATE/ALTER TABLE
* Build a CREATE SEQUENCE command to create the
* sequence object, and add it to the list of things
* to be done before this CREATE/ALTER TABLE
*/
seqstmt = makeNode(CreateSeqStmt);
seqstmt->with_rowid = true;
seqstmt->sequence = makeRangeVar(snamespace, sname, -1);
seqstmt->options = lcons(makeDefElem("as",
(Node *) makeTypeNameFromOid(INT8OID, -1),
-1), seqstmt->options);
(Node *) makeTypeNameFromOid(INT8OID, -1),
-1), seqstmt->options);
if (rowid_seq_cache > 1)
{
seqstmt->options = lcons(makeDefElem("cache",
(Node *) makeInteger(rowid_seq_cache),
-1), seqstmt->options);
(Node *) makeInteger(rowid_seq_cache),
-1), seqstmt->options);
}
else
seqstmt->options = lcons(makeDefElem("nocache",
NULL,
-1),
seqstmt->options);
NULL,
-1),
seqstmt->options);

if (cxt.rel)
seqstmt->ownerId = cxt.rel->rd_rel->relowner;

cxt.blist = lappend(cxt.blist, seqstmt);

/*
* Build an ALTER SEQUENCE ... OWNED BY command to mark the sequence as owned
* by this table.
* Build an ALTER SEQUENCE ... OWNED BY command to
* mark the sequence as owned by this table.
*/
altseqstmt = makeNode(AlterSeqStmt);
altseqstmt->sequence = makeRangeVar(snamespace, sname, -1);

relnamelist = list_make3(makeString(snamespace),
makeString(cxt.relation->relname),
makeString(sname));
makeString(cxt.relation->relname),
makeString(sname));

altseqstmt->options = list_make1(makeDefElem("owned_by",
Comment on lines 3834 to 3838
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 | 🔴 Critical

Also fix OWNED BY clause in ALTER TABLE path.

The same issue occurs here: relnamelist uses the sequence name (sname) as the column identifier, so the generated ALTER SEQUENCE ... OWNED BY still targets a non-existent column. Update this block to pass the ROWID column name instead.

-                       relnamelist = list_make3(makeString(snamespace),
-                                                 makeString(cxt->relation->relname),
-                                                 makeString(sname));
+                       relnamelist = list_make3(makeString(snamespace),
+                                                 makeString(cxt->relation->relname),
+                                                 makeString("rowid"));

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/backend/parser/parse_utilcmd.c around lines 3834-3838, relnamelist is
currently built with makeString(sname) and the ALTER SEQUENCE OWNED BY option
also references the sequence name as the column; replace those uses with the
table's ROWID column name. Specifically, change the third element in relnamelist
from makeString(sname) to makeString(cxt.rowidcol) (or the exact field holding
the ROWID column name) and ensure the owned_by option is set to refer to that
same ROWID column name instead of sname so ALTER SEQUENCE ... OWNED BY targets
the correct column.

(Node *) relnamelist, -1));
(Node *) relnamelist, -1));
cxt.alist = lappend(cxt.alist, altseqstmt);

newcmds = lappend(newcmds, cmd);
Expand Down Expand Up @@ -4669,7 +4678,7 @@ transformPartitionRangeBounds(ParseState *pstate, List *blist,
* as ColumnRefs.
*/
if (IsA(expr, ColumnRef) ||
IsA(expr, ColumnRefOrFuncCall))
IsA(expr, ColumnRefOrFuncCall))
{
ColumnRef *cref = NULL;
char *cname = NULL;
Expand Down
12 changes: 11 additions & 1 deletion src/backend/utils/misc/ivy_guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,19 @@ static struct config_bool Ivy_ConfigureNamesBool[] =
NULL, NULL, NULL
},

/*
* ivorysql.default_with_rowids
*
* When enabled, all newly created tables will automatically include
* an Oracle-compatible ROWID pseudo-column. This provides compatibility
* with Oracle applications that rely on ROWID for row identification.
*
* Default: off
* Context: USERSET (can be changed by any user)
*/
{
{"ivorysql.default_with_rowids", PGC_USERSET, DEVELOPER_OPTIONS,
gettext_noop("Auto add rowid column for create new table."),
gettext_noop("Automatically add rowid column when creating new tables."),
NULL,
GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE
},
Expand Down
2 changes: 1 addition & 1 deletion src/include/access/htup_details.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ struct HeapTupleHeaderData
#define HEAP_HASNULL 0x0001 /* has null attribute(s) */
#define HEAP_HASVARWIDTH 0x0002 /* has variable-width attribute(s) */
#define HEAP_HASEXTERNAL 0x0004 /* has external stored attribute(s) */
#define HEAP_HASROWID 0x0008 /* has an ROWID field */
#define HEAP_HASROWID 0x0008 /* has a ROWID field */
#define HEAP_XMAX_KEYSHR_LOCK 0x0010 /* xmax is a key-shared locker */
#define HEAP_COMBOCID 0x0020 /* t_cid is a combo CID */
#define HEAP_XMAX_EXCL_LOCK 0x0040 /* xmax is exclusive locker */
Expand Down
6 changes: 3 additions & 3 deletions src/oracle_test/regress/sql/ora_rowid.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--
-- Test Compatible Oracle ROWID pseudo
-- Test Oracle-Compatible ROWID Pseudo-Column
--

--
Expand Down Expand Up @@ -111,12 +111,12 @@ alter table t2 set without rowid;

select (rowid).rowno, * from t2;

-- clearup
-- cleanup
drop table t2;
drop table t3;

--
--3, CTEATE TABLE ... WITH ROWID
--3, CREATE TABLE ... WITH ROWID
--
create table t2 (a int, b int) with rowid;

Expand Down