Skip to content

Commit 586f5d0

Browse files
andinuxandinux
authored
perf(postgres): use SQL template casting for non-PK column types (#4)
* perf(postgres): use SQL template casting for non-PK column types Refactor type conversion for non-PK columns to use SQL template casting ($n::typename) instead of per-value SPI_execute_with_args("SELECT $1::typename") calls. This matches the pattern already used for primary key columns. Changes: - sql_postgresql.c: Add format_type() lookup and $n::coltype casting to SQL_BUILD_UPSERT_PK_AND_COL for non-PK column values - cloudsync_postgresql.c: Simplify cloudsync_decode_bytea_to_pgvalue() to return base types only (INT8, FLOAT8, TEXT, BYTEA) without SPI casting - cloudsync_postgresql.c: Remove lookup_column_type_oid() function - Add test 19 (19_uuid_pk_with_unmapped_cols.sql) covering UUID PK with unmapped non-PK types (JSONB, UUID, INET, CIDR) and all CRUD operations: INSERT, UPDATE non-PK, UPDATE mapped cols, DELETE, RESURRECT, UPDATE PK Performance benefit: Eliminates one SPI_execute call per non-PK column value during payload application, reducing overhead for unmapped PostgreSQL types. * test(postgres): new tests
1 parent 16d967d commit 586f5d0

File tree

8 files changed

+956
-95
lines changed

8 files changed

+956
-95
lines changed

‎src/cloudsync.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
extern "C" {
1818
#endif
1919

20-
#define CLOUDSYNC_VERSION "0.9.100"
20+
#define CLOUDSYNC_VERSION "0.9.101"
2121
#define CLOUDSYNC_MAX_TABLENAME_LEN 512
2222

2323
#define CLOUDSYNC_VALUE_NOTSET -1

‎src/postgresql/cloudsync_postgresql.c‎

Lines changed: 18 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,8 +1638,9 @@ static int cloudsync_decode_value_cb (void *xdata, int index, int type, int64_t
16381638
return DBRES_OK;
16391639
}
16401640

1641-
// Decode encoded bytea into a pgvalue_t matching the target type
1642-
static pgvalue_t *cloudsync_decode_bytea_to_pgvalue (bytea *encoded, Oid target_typoid, const char *target_typname, bool *out_isnull) {
1641+
// Decode encoded bytea into a pgvalue_t with the decoded base type.
1642+
// Type casting to the target column type is handled by the SQL statement.
1643+
static pgvalue_t *cloudsync_decode_bytea_to_pgvalue (bytea *encoded, bool *out_isnull) {
16431644
// Decode input guardrails.
16441645
if (out_isnull) *out_isnull = true;
16451646
if (!encoded) return NULL;
@@ -1652,37 +1653,34 @@ static pgvalue_t *cloudsync_decode_bytea_to_pgvalue (bytea *encoded, Oid target_
16521653
if (out_isnull) *out_isnull = dv.isnull;
16531654
if (dv.isnull) return NULL;
16541655

1655-
// Map decoded C types into a PostgreSQL Datum.
1656-
Oid argt[1] = {TEXTOID};
1657-
Datum argv[1];
1658-
char argn[1] = {' '};
1659-
bool argv_is_pointer = false; // Track if argv[0] needs pfree on error
1656+
// Map decoded C types into a PostgreSQL Datum with the base type.
1657+
// The SQL statement handles casting to the target column type via $n::typename.
1658+
Oid typoid = TEXTOID;
1659+
Datum datum;
16601660

16611661
switch (dv.dbtype) {
16621662
case DBTYPE_INTEGER:
1663-
argt[0] = INT8OID;
1664-
argv[0] = Int64GetDatum(dv.ival);
1663+
typoid = INT8OID;
1664+
datum = Int64GetDatum(dv.ival);
16651665
break;
16661666
case DBTYPE_FLOAT:
1667-
argt[0] = FLOAT8OID;
1668-
argv[0] = Float8GetDatum(dv.dval);
1667+
typoid = FLOAT8OID;
1668+
datum = Float8GetDatum(dv.dval);
16691669
break;
16701670
case DBTYPE_TEXT: {
1671-
argt[0] = TEXTOID;
1671+
typoid = TEXTOID;
16721672
Size tlen = dv.pval ? (Size)dv.len : 0;
16731673
text *t = (text *)palloc(VARHDRSZ + tlen);
16741674
SET_VARSIZE(t, VARHDRSZ + tlen);
16751675
if (tlen > 0) memmove(VARDATA(t), dv.pval, tlen);
1676-
argv[0] = PointerGetDatum(t);
1677-
argv_is_pointer = true;
1676+
datum = PointerGetDatum(t);
16781677
} break;
16791678
case DBTYPE_BLOB: {
1680-
argt[0] = BYTEAOID;
1679+
typoid = BYTEAOID;
16811680
bytea *ba = (bytea *)palloc(VARHDRSZ + dv.len);
16821681
SET_VARSIZE(ba, VARHDRSZ + dv.len);
16831682
if (dv.len > 0) memcpy(VARDATA(ba), dv.pval, (size_t)dv.len);
1684-
argv[0] = PointerGetDatum(ba);
1685-
argv_is_pointer = true;
1683+
datum = PointerGetDatum(ba);
16861684
} break;
16871685
case DBTYPE_NULL:
16881686
if (out_isnull) *out_isnull = true;
@@ -1695,44 +1693,7 @@ static pgvalue_t *cloudsync_decode_bytea_to_pgvalue (bytea *encoded, Oid target_
16951693

16961694
if (dv.pval) pfree(dv.pval);
16971695

1698-
// Cast to the target column type from the table schema.
1699-
if (argt[0] == target_typoid) {
1700-
pgvalue_t *result = pgvalue_create(argv[0], target_typoid, -1, InvalidOid, false);
1701-
if (!result && argv_is_pointer) {
1702-
pfree(DatumGetPointer(argv[0]));
1703-
}
1704-
return result;
1705-
}
1706-
1707-
StringInfoData castq;
1708-
initStringInfo(&castq);
1709-
appendStringInfo(&castq, "SELECT $1::%s", target_typname);
1710-
1711-
int rc = SPI_execute_with_args(castq.data, 1, argt, argv, argn, true, 1);
1712-
if (rc != SPI_OK_SELECT || SPI_processed != 1 || !SPI_tuptable) {
1713-
if (SPI_tuptable) SPI_freetuptable(SPI_tuptable);
1714-
pfree(castq.data);
1715-
if (argv_is_pointer) pfree(DatumGetPointer(argv[0]));
1716-
ereport(ERROR, (errmsg("cloudsync: failed to cast value to %s", target_typname)));
1717-
}
1718-
pfree(castq.data);
1719-
1720-
bool typed_isnull = false;
1721-
// SPI_getbinval uses 1-based column indexing, but TupleDescAttr uses 0-based indexing
1722-
Datum typed_value = SPI_getbinval(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1, &typed_isnull);
1723-
int32 typmod = TupleDescAttr(SPI_tuptable->tupdesc, 0)->atttypmod;
1724-
Oid collation = TupleDescAttr(SPI_tuptable->tupdesc, 0)->attcollation;
1725-
if (!typed_isnull) {
1726-
Form_pg_attribute att = TupleDescAttr(SPI_tuptable->tupdesc, 0);
1727-
typed_value = datumCopy(typed_value, att->attbyval, att->attlen);
1728-
}
1729-
if (SPI_tuptable) {
1730-
SPI_freetuptable(SPI_tuptable);
1731-
SPI_tuptable = NULL;
1732-
}
1733-
1734-
if (out_isnull) *out_isnull = typed_isnull;
1735-
return pgvalue_create(typed_value, target_typoid, typmod, collation, typed_isnull);
1696+
return pgvalue_create(datum, typoid, -1, InvalidOid, false);
17361697
}
17371698

17381699
PG_FUNCTION_INFO_V1(cloudsync_encode_value);
@@ -2092,30 +2053,6 @@ static char * build_union_sql (void) {
20922053
return result;
20932054
}
20942055

2095-
static Oid lookup_column_type_oid (const char *tbl, const char *col_name, const char *schema) {
2096-
// SPI_connect not needed here
2097-
if (strcmp(col_name, CLOUDSYNC_TOMBSTONE_VALUE) == 0) return BYTEAOID;
2098-
2099-
// lookup table OID with optional schema qualification
2100-
Oid relid;
2101-
if (schema) {
2102-
Oid nspid = get_namespace_oid(schema, false);
2103-
relid = get_relname_relid(tbl, nspid);
2104-
} else {
2105-
relid = RelnameGetRelid(tbl);
2106-
}
2107-
if (!OidIsValid(relid)) ereport(ERROR, (errmsg("cloudsync: table \"%s\" not found (schema: %s)", tbl, schema ? schema : "search_path")));
2108-
2109-
// find attribute
2110-
int attnum = get_attnum(relid, col_name);
2111-
if (attnum == InvalidAttrNumber) ereport(ERROR, (errmsg("cloudsync: column \"%s\" not found in table \"%s\"", col_name, tbl)));
2112-
2113-
Oid typoid = get_atttype(relid, attnum);
2114-
if (!OidIsValid(typoid)) ereport(ERROR, (errmsg("cloudsync: could not resolve type for %s.%s", tbl, col_name)));
2115-
2116-
return typoid;
2117-
}
2118-
21192056
PG_FUNCTION_INFO_V1(cloudsync_changes_select);
21202057
Datum cloudsync_changes_select(PG_FUNCTION_ARGS) {
21212058
FuncCallContext *funcctx;
@@ -2307,19 +2244,12 @@ Datum cloudsync_changes_insert_trigger (PG_FUNCTION_ARGS) {
23072244
cloudsync_table_context *table = table_lookup(data, insert_tbl);
23082245
if (!table) ereport(ERROR, (errmsg("Unable to find table")));
23092246

2310-
// get real column type from tbl.col_name (skip tombstone sentinel)
2311-
Oid target_typoid = InvalidOid;
2312-
char *target_typname = NULL;
2313-
if (!is_tombstone) {
2314-
target_typoid = lookup_column_type_oid(insert_tbl, insert_name, cloudsync_schema(data));
2315-
target_typname = format_type_be(target_typoid);
2316-
}
2317-
23182247
if (SPI_connect() != SPI_OK_CONNECT) ereport(ERROR, (errmsg("cloudsync: SPI_connect failed in trigger")));
23192248
spi_connected = true;
23202249

2250+
// Decode value to base type; SQL statement handles type casting via $n::typename
23212251
if (!is_tombstone) {
2322-
col_value = cloudsync_decode_bytea_to_pgvalue(insert_value_encoded, target_typoid, target_typname, NULL);
2252+
col_value = cloudsync_decode_bytea_to_pgvalue(insert_value_encoded, NULL);
23232253
}
23242254

23252255
int rc = DBRES_OK;

‎src/postgresql/database_postgresql.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ char *sql_build_upsert_pk_and_col (cloudsync_context *data, const char *table_na
199199
char *qualified = database_build_base_ref(schema, table_name);
200200
if (!qualified) return NULL;
201201

202-
char *sql = cloudsync_memory_mprintf(SQL_BUILD_UPSERT_PK_AND_COL, qualified, colname);
202+
char *sql = cloudsync_memory_mprintf(SQL_BUILD_UPSERT_PK_AND_COL, qualified, colname, colname);
203203
cloudsync_memory_free(qualified);
204204
if (!sql) return NULL;
205205

‎src/postgresql/sql_postgresql.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ const char * const SQL_BUILD_SELECT_NONPK_COLS_BY_PK =
134134
" SELECT to_regclass('%s') AS oid"
135135
"), "
136136
"pk AS ("
137-
" SELECT a.attname, k.ord "
137+
" SELECT a.attname, k.ord, format_type(a.atttypid, a.atttypmod) AS coltype "
138138
" FROM pg_index x "
139139
" JOIN tbl t ON t.oid = x.indrelid "
140140
" JOIN LATERAL unnest(x.indkey) WITH ORDINALITY AS k(attnum, ord) ON true "
@@ -161,7 +161,7 @@ const char * const SQL_BUILD_SELECT_NONPK_COLS_BY_PK =
161161
" || (SELECT string_agg(format('%%I', attname), ',') FROM nonpk)"
162162
" || ' FROM ' || (SELECT (oid::regclass)::text FROM tbl)"
163163
" || ' WHERE '"
164-
" || (SELECT string_agg(format('%%I=$%%s', attname, ord), ' AND ' ORDER BY ord) FROM pk)"
164+
" || (SELECT string_agg(format('%%I=$%%s::%%s', attname, ord, coltype), ' AND ' ORDER BY ord) FROM pk)"
165165
" || ';';";
166166

167167
const char * const SQL_DELETE_ROW_BY_ROWID =
@@ -229,17 +229,20 @@ const char * const SQL_BUILD_UPSERT_PK_AND_COL =
229229
" SELECT count(*) AS n FROM pk"
230230
"), "
231231
"col AS ("
232-
" SELECT '%s'::text AS colname"
232+
" SELECT '%s'::text AS colname, format_type(a.atttypid, a.atttypmod) AS coltype "
233+
" FROM pg_attribute a "
234+
" JOIN tbl t ON t.oid = a.attrelid "
235+
" WHERE a.attname = '%s' AND a.attnum > 0 AND NOT a.attisdropped"
233236
") "
234237
"SELECT "
235238
" 'INSERT INTO ' || (SELECT (oid::regclass)::text FROM tbl)"
236239
" || ' (' || (SELECT string_agg(format('%%I', attname), ',') FROM pk)"
237240
" || ',' || (SELECT format('%%I', colname) FROM col) || ')'"
238241
" || ' VALUES (' || (SELECT string_agg(format('$%%s::%%s', ord, coltype), ',') FROM pk)"
239-
" || ',' || (SELECT format('$%%s', (SELECT n FROM pk_count) + 1)) || ')'"
242+
" || ',' || (SELECT format('$%%s::%%s', (SELECT n FROM pk_count) + 1, coltype) FROM col) || ')'"
240243
" || ' ON CONFLICT (' || (SELECT string_agg(format('%%I', attname), ',') FROM pk) || ')'"
241244
" || ' DO UPDATE SET ' || (SELECT format('%%I', colname) FROM col)"
242-
" || '=' || (SELECT format('$%%s', (SELECT n FROM pk_count) + 2)) || ';';";
245+
" || '=' || (SELECT format('$%%s::%%s', (SELECT n FROM pk_count) + 2, coltype) FROM col) || ';';";
243246

244247
const char * const SQL_SELECT_COLS_BY_ROWID_FMT =
245248
"SELECT %s%s%s FROM %s WHERE ctid = $1;"; // TODO: align with PK/rowid selection builder

‎test/postgresql/01_unittest.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ SELECT cloudsync_version() AS version \gset
2121

2222
-- Test uuid generation
2323
SELECT cloudsync_uuid() AS uuid1 \gset
24+
SELECT pg_sleep(0.1);
2425
SELECT cloudsync_uuid() AS uuid2 \gset
2526

2627
-- Test 1: Format check (UUID v7 has standard format: xxxxxxxx-xxxx-7xxx-xxxx-xxxxxxxxxxxx)

0 commit comments

Comments
 (0)