Skip to content

Commit

Permalink
Resolve issue #4: Improve the value data type handling in the add fun…
Browse files Browse the repository at this point in the history
…ction, add a value_type function to make it easier to see the collection type and make the README more clear how types are handled with subscripts.
  • Loading branch information
jim-mlodgenski committed Jan 30, 2025
1 parent b735f60 commit 5883ac1
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 22 deletions.
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ CREATE EXTENSION collection;
| values_to_table(collection, anyelement) | SETOF anyelement | Returns all of the values as anyelement in the collection |
| to_table(collection) | TABLE(text, text) | Returns all of the keys and values as text in the collection |
| to_table(collection, anyelement) | TABLE(text, anyelement) | Returns all of the keys and values as anyelement in the collection |
| value_type(collection) | regtype | Returns the data type of the elements within the collection |

## Using Subscripts

Expand Down Expand Up @@ -125,7 +126,9 @@ The default type of a collection's element is `text`, however it can contain
any valid PostgreSQL type. The type can be set in two ways. The first is by
explicitly setting it as the type modifier when declaring the collection. If
no type modifier is defined, the type of the first element added to the
collection will define the type of the collection.
collection using the `add` function will define the type of the collection.
If no type modifier is set, any subscript assignments or fetches will be cast
to type `text`.

```sql
DO
Expand Down
5 changes: 5 additions & 0 deletions sql/collection--0.9.sql
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ CREATE FUNCTION to_table(collection, anyelement, OUT key text, OUT value anyelem
AS 'MODULE_PATHNAME', 'collection_to_table'
LANGUAGE c;

CREATE FUNCTION value_type(collection)
RETURNS regtype
AS 'MODULE_PATHNAME', 'collection_value_type'
LANGUAGE c;

CREATE FUNCTION collection_stats(OUT add int8, OUT context_switch int8,
OUT delete int8, OUT find int8, OUT sort int8)
AS 'MODULE_PATHNAME', 'collection_stats'
Expand Down
15 changes: 9 additions & 6 deletions src/collection_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "catalog/pg_type.h"
#include "common/jsonapi.h"
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "utils/builtins.h"
#include "utils/datum.h"
Expand Down Expand Up @@ -184,24 +185,26 @@ Datum
collection_cast(PG_FUNCTION_ARGS)
{
CollectionHeader *colhdr;
int32 typmod = PG_GETARG_INT32(1);
Oid typmod = PG_GETARG_INT32(1);

colhdr = fetch_collection(fcinfo, 0);

pgstat_report_wait_start(collection_we_cast);

if (typmod > 0 && colhdr->value_type != InvalidOid)
{
Oid value_type = typmod;

if (colhdr->value_type != value_type)
/*
* Check if the cast is into a collection or if it can be coerced to
* the appropriate type
*/
if (get_fn_expr_argtype(fcinfo->flinfo, 0) != typmod &&
!can_coerce_type(1, &colhdr->value_type, &typmod, COERCION_IMPLICIT))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("Incompatible value data type"),
errdetail("Expecting %s, but received %s",
format_type_extended(value_type, -1, 0),
format_type_extended(typmod, -1, 0),
format_type_extended(colhdr->value_type, -1, 0))));

}

pgstat_report_wait_end();
Expand Down
50 changes: 41 additions & 9 deletions src/collection_subs.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ collection_subscript_transform(SubscriptingRef *sbsref,
sbsref->refrestype = TEXTOID;
else
sbsref->refrestype = sbsref->reftypmod;

sbsref->reftypmod = -1;
}

/*
Expand All @@ -114,6 +112,7 @@ collection_subscript_fetch(ExprState *state,
ExprContext *econtext)
{
SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
CollectionSubWorkspace *workspace = (CollectionSubWorkspace *) sbsrefstate->workspace;
CollectionHeader *colhdr;
collection *item;
char *key;
Expand Down Expand Up @@ -141,8 +140,30 @@ collection_subscript_fetch(ExprState *state,
if (item == NULL)
value = (Datum) 0;
else
value = datumCopy(item->value, colhdr->value_byval, colhdr->value_type_len);

{
if (can_coerce_type(1, &workspace->value_type, &colhdr->value_type, COERCION_IMPLICIT))
value = datumCopy(item->value, colhdr->value_byval, colhdr->value_type_len);
else
{
if (workspace->value_type == TEXTOID)
{
bool typisvarlena;
Oid outfuncoid;

getTypeOutputInfo(colhdr->value_type, &outfuncoid, &typisvarlena);
value = CStringGetTextDatum(DatumGetCString(OidFunctionCall1(outfuncoid, item->value)));
}
else
{
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("Incompatible value data type"),
errdetail("Expecting %s, but received %s",
format_type_extended(workspace->value_type, -1, 0),
format_type_extended(colhdr->value_type, -1, 0))));
}
}
}

if (value == (Datum) 0)
*op->resnull = true;
Expand Down Expand Up @@ -185,6 +206,10 @@ collection_subscript_assign(ExprState *state,
{
colhdr = construct_empty_collection(CurrentMemoryContext);
*op->resnull = false;

colhdr->value_type = workspace->value_type;
colhdr->value_type_len = workspace->value_type_len;
colhdr->value_byval = workspace->value_byval;
}
else
{
Expand All @@ -193,6 +218,17 @@ collection_subscript_assign(ExprState *state,

pgstat_report_wait_start(collection_we_assign);

if (!can_coerce_type(1, &workspace->value_type, &colhdr->value_type, COERCION_IMPLICIT))
{
pgstat_report_wait_end();
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("incompatible value data type"),
errdetail("expecting %s, but received %s",
format_type_extended(colhdr->value_type, -1, 0),
format_type_extended(workspace->value_type, -1, 0))));
}

oldcxt = MemoryContextSwitchTo(colhdr->hdr.eoh_context);

key = text_to_cstring(DatumGetTextPP(sbsrefstate->upperindex[0]));
Expand All @@ -203,10 +239,6 @@ collection_subscript_assign(ExprState *state,

HASH_REPLACE(hh, colhdr->current, key[0], strlen(key), item, replaced_item);

colhdr->value_type = workspace->value_type;
colhdr->value_type_len = workspace->value_type_len;
colhdr->value_byval = workspace->value_byval;

if (colhdr->head == NULL)
colhdr->head = colhdr->current;

Expand Down Expand Up @@ -240,7 +272,7 @@ collection_exec_setup(const SubscriptingRef *sbsref,
workspace = (CollectionSubWorkspace *) palloc(sizeof(CollectionSubWorkspace));
sbsrefstate->workspace = workspace;

/* Default to storing text unless the typmod is set */
/* Default to fetching as text unless the typmod is set */
if (sbsref->reftypmod == -1)
{
workspace->value_type = TEXTOID;
Expand Down
41 changes: 37 additions & 4 deletions src/collection_userfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ PG_FUNCTION_INFO_V1(collection_keys_to_table);
PG_FUNCTION_INFO_V1(collection_values_to_table);
PG_FUNCTION_INFO_V1(collection_to_table);

PG_FUNCTION_INFO_V1(collection_value_type);
PG_FUNCTION_INFO_V1(collection_stats);
PG_FUNCTION_INFO_V1(collection_stats_reset);

Expand Down Expand Up @@ -88,16 +89,35 @@ collection_add(PG_FUNCTION_ARGS)
argtype = get_fn_expr_argtype(fcinfo->flinfo, 2);
get_typlenbyval(argtype, &argtypelen, &argtypebyval);

/*
* Set the value type of the collection to the first element added
*/
if (colhdr->value_type == InvalidOid)
{
colhdr->value_type = argtype;
colhdr->value_type_len = argtypelen;
colhdr->value_byval = argtypebyval;
}
else
{
if (!can_coerce_type(1, &argtype, &colhdr->value_type, COERCION_IMPLICIT))
{
pgstat_report_wait_end();
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("incompatible value data type"),
errdetail("expecting %s, but received %s",
format_type_extended(colhdr->value_type, -1, 0),
format_type_extended(argtype, -1, 0))));
}
}

item = (collection *) palloc(sizeof(collection));
item->key = key;
item->value = datumCopy(value, argtypebyval, argtypelen);

HASH_REPLACE(hh, colhdr->current, key[0], strlen(key), item, replaced_item);

colhdr->value_type = argtype;
colhdr->value_type_len = argtypelen;
colhdr->value_byval = argtypebyval;

if (colhdr->head == NULL)
colhdr->head = colhdr->current;

Expand Down Expand Up @@ -589,6 +609,19 @@ collection_to_table(PG_FUNCTION_ARGS)
}
}

Datum
collection_value_type(PG_FUNCTION_ARGS)
{
CollectionHeader *colhdr;

colhdr = fetch_collection(fcinfo, 0);

if (colhdr->head == NULL || colhdr->value_type == InvalidOid)
PG_RETURN_NULL();

PG_RETURN_OID(colhdr->value_type);
}

Datum
collection_stats(PG_FUNCTION_ARGS)
{
Expand Down
41 changes: 41 additions & 0 deletions test/expected/collection.out
Original file line number Diff line number Diff line change
Expand Up @@ -396,3 +396,44 @@ SELECT * FROM collections_test ORDER BY c1;
(2 rows)

DROP TABLE collections_test;
DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Test 23';
t := add(t, '1', 111::bigint);
t := add(t, '2', 222::bigint);
RAISE NOTICE 'The current val is %', value(t, null::bigint);
RAISE NOTICE 'The current value type is %', pg_typeof(value(t, null::bigint));
END
$$;
NOTICE: Test 23
NOTICE: The current val is 111
NOTICE: The current value type is bigint
DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Test 24';
t := add(t, '1', 111::bigint);
t := add(t, '2', 'hello'::text);
END
$$;
NOTICE: Test 24
ERROR: incompatible value data type
DETAIL: expecting bigint, but received text
CONTEXT: PL/pgSQL function inline_code_block line 7 at assignment
DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Test 25';
t := add(t, '1', 111::bigint);
RAISE NOTICE 'The type is %', value_type(t);
END
$$;
NOTICE: Test 25
NOTICE: The type is bigint
33 changes: 31 additions & 2 deletions test/expected/subscript.out
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ BEGIN
END
$$;
NOTICE: Subscript test 10
ERROR: Incompatible value data type
DETAIL: Expecting text, but received date
ERROR: incompatible value data type
DETAIL: expecting text, but received date
CONTEXT: PL/pgSQL function inline_code_block line 7 at assignment
DO
$$
Expand Down Expand Up @@ -176,3 +176,32 @@ END
$$;
NOTICE: Subscript test 12
NOTICE: The current val is <NULL>
DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Subscript test 13';

t['1'] := 111::bigint;
t['2'] := 222::bigint;
RAISE NOTICE 'The type is %', value_type(t);
END
$$;
NOTICE: Subscript test 13
NOTICE: The type is text
DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Subscript test 14';

t := add(t, '1', 111::bigint);
t['2'] := 'abc';
END
$$;
NOTICE: Subscript test 14
ERROR: incompatible value data type
DETAIL: expecting bigint, but received text
CONTEXT: PL/pgSQL function inline_code_block line 8 at assignment
35 changes: 35 additions & 0 deletions test/sql/collection.sql
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,38 @@ INSERT INTO collections_test VALUES (2, add(null::collection, 'bbb', 'Hello ALL'
SELECT * FROM collections_test ORDER BY c1;

DROP TABLE collections_test;

DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Test 23';
t := add(t, '1', 111::bigint);
t := add(t, '2', 222::bigint);
RAISE NOTICE 'The current val is %', value(t, null::bigint);
RAISE NOTICE 'The current value type is %', pg_typeof(value(t, null::bigint));
END
$$;

DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Test 24';
t := add(t, '1', 111::bigint);
t := add(t, '2', 'hello'::text);
END
$$;

DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Test 25';
t := add(t, '1', 111::bigint);
RAISE NOTICE 'The type is %', value_type(t);
END
$$;
27 changes: 27 additions & 0 deletions test/sql/subscript.sql
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,30 @@ BEGIN
RAISE NOTICE 'The current val is %', t['2'];
END
$$;

DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Subscript test 13';

t['1'] := 111::bigint;
t['2'] := 222::bigint;
RAISE NOTICE 'The type is %', value_type(t);
END
$$;


DO
$$
DECLARE
t collection;
BEGIN
RAISE NOTICE 'Subscript test 14';

t := add(t, '1', 111::bigint);
t['2'] := 'abc';
END
$$;

0 comments on commit 5883ac1

Please sign in to comment.