public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v41 2/2] Filling gaps in jsonb
7+ messages / 4 participants
[nested] [flat]

* [PATCH v41 2/2] Filling gaps in jsonb
@ 2020-12-31 14:19 Dmitrii Dolgov <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Dmitrii Dolgov @ 2020-12-31 14:19 UTC (permalink / raw)

Introduces two new modes for jsonb assignment:

* Appending array elements on the specified position, gaps filled with
nulls (similar to JavaScript behavior). This mode also instructs to
create the whole path in a jsonb object if some part of the path (more
than just the last element) is not present.

* Assigning keeps array positions consistent by prevent prepending of
elements.

Originally proposed by Nikita Glukhov based on polymorphic subscripting
patch, but transformed into an independent change.
---
 doc/src/sgml/json.sgml              |  33 +++++
 src/backend/utils/adt/jsonfuncs.c   | 207 +++++++++++++++++++++++++++-
 src/test/regress/expected/jsonb.out |  85 ++++++++++++
 src/test/regress/sql/jsonb.sql      |  50 +++++++
 4 files changed, 369 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/json.sgml b/doc/src/sgml/json.sgml
index 100d1a60f4..9af015d222 100644
--- a/doc/src/sgml/json.sgml
+++ b/doc/src/sgml/json.sgml
@@ -645,6 +645,39 @@ UPDATE table_name SET jsonb_field['a'] = '1';
 
 -- If jsonb_field here is NULL, the result is [1]
 UPDATE table_name SET jsonb_field[0] = '1';
+</programlisting>
+
+   Jsonb assignment via subscripting handles few edge cases differently
+   from <literal>jsonb_set</literal>. When assigning to the jsonb array
+   to the specified index, but there are no other elements present, the
+   result will be a jsonb array with the ewn value by specified index and
+   <type>null</type> elements from the first index to the specified index.
+
+<programlisting>
+-- If jsonb_field is [], the result is [null, null, 2]
+UPDATE table_name SET jsonb_field[2] = '2';
+</programlisting>
+
+   When assigning to the jsonb array to the specified index, but position
+   of the last element in the array is less than the specified index, the
+   result will be a jsonb array with the new value by specified index and
+   <type>null</type> elements from the last index to the specified index.
+
+<programlisting>
+-- If jsonb_field is [0], the result is [0, null, 2]
+UPDATE table_name SET jsonb_field[2] = '2';
+</programlisting>
+
+   When assigning using the path which is not present in the source jsonb,
+   the result will be a jsonb with the specified path created and the new
+   value at the end of the path.
+
+<programlisting>
+-- If jsonb_field is {}, the result is {'a': [{'b': 1}]}
+UPDATE table_name SET jsonb_field['a'][0]['b'] = '1';
+
+-- If jsonb_field is [], the result is [{'a': 1}]
+UPDATE table_name SET jsonb_field[0]['a'] = '1';
 </programlisting>
 
   </para>
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 5a0ba6b220..c8edc8db4c 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -44,6 +44,8 @@
 #define JB_PATH_INSERT_AFTER			0x0010
 #define JB_PATH_CREATE_OR_INSERT \
 	(JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER | JB_PATH_CREATE)
+#define JB_PATH_FILL_GAPS				0x0020
+#define JB_PATH_CONSISTENT_POSITION		0x0040
 
 /* state for json_object_keys */
 typedef struct OkeysState
@@ -1634,14 +1636,108 @@ jsonb_set_element(Jsonb* jb, Datum *path, int path_len,
 
 	it = JsonbIteratorInit(&jb->root);
 
-	res = setPath(&it, path, path_nulls, path_len, &state, 0,
-				  newval, JB_PATH_CREATE);
+	res = setPath(&it, path, path_nulls, path_len, &state, 0, newval,
+				  JB_PATH_CREATE | JB_PATH_FILL_GAPS |
+				  JB_PATH_CONSISTENT_POSITION);
 
 	pfree(path_nulls);
 
 	PG_RETURN_JSONB_P(JsonbValueToJsonb(res));
 }
 
+static void
+push_null_elements(JsonbParseState **ps, int num)
+{
+		JsonbValue	null;
+
+		null.type = jbvNull;
+
+		while (num-- > 0)
+				pushJsonbValue(ps, WJB_ELEM, &null);
+}
+
+/*
+ * Prepare a new structure containing nested empty objects and arrays
+ * corresponding to the specified path, and assign a new value at the end of
+ * this path. E.g. the path [a][0][b] with the new value 1 will produce the
+ * structure {a: [{b: 1}]}.
+ *
+ * Called is responsible to make sure such path does not exist yet.
+ */
+static void
+push_path(JsonbParseState **st, int level, Datum *path_elems,
+		  bool *path_nulls, int path_len, JsonbValue *newval)
+{
+	enum jbvType *tpath = palloc((path_len - level) * sizeof(enum jbvType));
+	long		 lindex;
+	JsonbValue	 newkey;
+
+	/* Create first part of the chain with beginning tokens */
+	for(int i = level + 1; i < path_len; i++)
+	{
+		char   	   *c, *badp;
+
+		if (path_nulls[i])
+			break;
+
+		/*
+		 * Try to convert to an integer to find out the expected type,
+		 * object or array.
+		 */
+		c = TextDatumGetCString(path_elems[i]);
+		errno = 0;
+		lindex = strtol(c, &badp, 10);
+		if (errno != 0 || badp == c || *badp != '\0' || lindex > INT_MAX ||
+			lindex < INT_MIN)
+		{
+			/* text, an object is expected */
+			newkey.type = jbvString;
+			newkey.val.string.len = VARSIZE_ANY_EXHDR(path_elems[i]);
+			newkey.val.string.val = VARDATA_ANY(path_elems[i]);
+
+			if (i > level)
+				(void) pushJsonbValue(st, WJB_BEGIN_OBJECT, NULL);
+
+			(void) pushJsonbValue(st, WJB_KEY, &newkey);
+
+			tpath[i] = jbvObject;
+		}
+		else
+		{
+			/* integer, an array is expected */
+			(void) pushJsonbValue(st, WJB_BEGIN_ARRAY, NULL);
+
+			push_null_elements(st, lindex);
+
+			tpath[i] = jbvArray;
+		}
+
+	}
+
+	/* Insert an actual value for either an object or array */
+	if (tpath[path_len - 1] == jbvArray)
+	{
+		(void) pushJsonbValue(st, WJB_ELEM, newval);
+	}
+	else
+		(void) pushJsonbValue(st, WJB_VALUE, newval);
+
+	/*
+	 * Close everything up to the last but one level. The last one will be
+	 * closed outside of this function.
+	 */
+	for(int i = path_len - 1; i > level; i--)
+	{
+		if (path_nulls[i])
+			break;
+
+		if (tpath[i] == jbvObject)
+			(void) pushJsonbValue(st, WJB_END_OBJECT, NULL);
+		else
+			(void) pushJsonbValue(st, WJB_END_ARRAY, NULL);
+	}
+}
+
 /*
  * Return the text representation of the given JsonbValue.
  */
@@ -4782,6 +4878,21 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
  * Bits JB_PATH_INSERT_BEFORE and JB_PATH_INSERT_AFTER in op_type
  * behave as JB_PATH_CREATE if new value is inserted in JsonbObject.
  *
+ * If JB_PATH_FILL_GAPS bit is set, this will change an assignment logic in
+ * case if target is an array. The assignment index will not be restricted by
+ * number of elements in the array, and if there are any empty slots between
+ * last element of the array and a new one they will be filled with nulls. If
+ * the index is negative, it still will be considered an an index from the end
+ * of the array. Of a part of the path is not present and this part is more
+ * than just one last element, this flag will instruct to create the whole
+ * chain of corresponding objects and insert the value.
+ *
+ * JB_PATH_CONSISTENT_POSITION for an array indicates that the called wants to
+ * keep values with fixed indices. Indices for existing elements could be
+ * changed (shifted forward) in case if the array is prepended with a new value
+ * and a negative index out of the range, so this behavior will be prevented
+ * and return an error.
+ *
  * All path elements before the last must already exist
  * whatever bits in op_type are set, or nothing is done.
  */
@@ -4940,6 +5051,35 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			}
 		}
 	}
+
+	/*
+	 * If we got here there are only few possibilities:
+	 * - no target path was found, and an open object with some keys/values was
+	 *   pushed into the state
+	 * - an object is empty, nothing is pushed into the state yet
+	 *
+	 * In both cases if instructed to create the path when not present,
+	 * generate the whole chain of empty objects and insert the new value
+	 * there.
+	 */
+	if (!done && (op_type & JB_PATH_FILL_GAPS) && (level < path_len - 1))
+	{
+		JsonbValue	 newkey;
+
+		/* If an object is currently empty, start a new one */
+		if (npairs == 0)
+			(void) pushJsonbValue(st, WJB_BEGIN_OBJECT, NULL);
+
+		newkey.type = jbvString;
+		newkey.val.string.len = VARSIZE_ANY_EXHDR(path_elems[level]);
+		newkey.val.string.val = VARDATA_ANY(path_elems[level]);
+
+		(void) pushJsonbValue(st, WJB_KEY, &newkey);
+		(void) push_path(st, level, path_elems, path_nulls,
+						 path_len, newval);
+
+		/* Result is closed with WJB_END_OBJECT outside of this function */
+	}
 }
 
 /*
@@ -4978,25 +5118,48 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 	if (idx < 0)
 	{
 		if (-idx > nelems)
-			idx = INT_MIN;
+		{
+			/*
+			 * If asked to keep elements position consistent, it's not allowed
+			 * to prepend the array.
+			 */
+			if (op_type & JB_PATH_CONSISTENT_POSITION)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("path element at position %d is out of range: %d",
+								level + 1, idx)));
+			else
+				idx = INT_MIN;
+		}
 		else
 			idx = nelems + idx;
 	}
 
-	if (idx > 0 && idx > nelems)
-		idx = nelems;
+	/*
+	 * Filling the gaps means there are no limits on the positive index are
+	 * imposed, we can set any element. Otherwise limit the index by nelems.
+	 */
+	if (!(op_type & JB_PATH_FILL_GAPS))
+	{
+		if (idx > 0 && idx > nelems)
+			idx = nelems;
+	}
 
 	/*
 	 * if we're creating, and idx == INT_MIN, we prepend the new value to the
 	 * array also if the array is empty - in which case we don't really care
 	 * what the idx value is
 	 */
-
 	if ((idx == INT_MIN || nelems == 0) && (level == path_len - 1) &&
 		(op_type & JB_PATH_CREATE_OR_INSERT))
 	{
 		Assert(newval != NULL);
+
+		if (op_type & JB_PATH_FILL_GAPS && nelems == 0 && idx > 0)
+			push_null_elements(st, idx);
+
 		(void) pushJsonbValue(st, WJB_ELEM, newval);
+
 		done = true;
 	}
 
@@ -5057,10 +5220,42 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			if ((op_type & JB_PATH_CREATE_OR_INSERT) && !done &&
 				level == path_len - 1 && i == nelems - 1)
 			{
+				/*
+				 * If asked to fill the gaps, idx could be bigger than nelems,
+				 * so prepend the new element with nulls if that's the case.
+				 */
+				if (op_type & JB_PATH_FILL_GAPS && idx > nelems)
+					push_null_elements(st, idx - nelems);
+
 				(void) pushJsonbValue(st, WJB_ELEM, newval);
 			}
 		}
 	}
+
+	/*
+	 * If we got here there are only few possibilities:
+	 * - no target path was found, and an open array with some keys/values was
+	 *   pushed into the state
+	 * - an array is empty, nothing is pushed into the state yet
+	 *
+	 * In both cases if instructed to create the path when not present,
+	 * generate the whole chain of empty objects and insert the new value
+	 * there.
+	 */
+	if (!done && (op_type & JB_PATH_FILL_GAPS) && (level < path_len - 1))
+	{
+		/* If an array is currently empty, start a new one */
+		if (nelems == 0)
+			(void) pushJsonbValue(st, WJB_BEGIN_ARRAY, NULL);
+
+		if (idx > 0)
+			push_null_elements(st, idx - nelems);
+
+		(void) push_path(st, level, path_elems, path_nulls,
+						 path_len, newval);
+
+		/* Result is closed with WJB_END_OBJECT outside of this function */
+	}
 }
 
 /*
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index bb3f25ec3f..07f570b751 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -4999,6 +4999,91 @@ select * from test_jsonb_subscript;
   3 | [1]
 (3 rows)
 
+-- Fill the gaps logic
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[0]');
+update test_jsonb_subscript set test_json[5] = '1';
+select * from test_jsonb_subscript;
+ id |           test_json            
+----+--------------------------------
+  1 | [0, null, null, null, null, 1]
+(1 row)
+
+update test_jsonb_subscript set test_json[-4] = '1';
+select * from test_jsonb_subscript;
+ id |          test_json          
+----+-----------------------------
+  1 | [0, null, 1, null, null, 1]
+(1 row)
+
+update test_jsonb_subscript set test_json[-8] = '1';
+ERROR:  path element at position 1 is out of range: -8
+select * from test_jsonb_subscript;
+ id |          test_json          
+----+-----------------------------
+  1 | [0, null, 1, null, null, 1]
+(1 row)
+
+-- keep consistent values position
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[]');
+update test_jsonb_subscript set test_json[5] = '1';
+select * from test_jsonb_subscript;
+ id |             test_json             
+----+-----------------------------------
+  1 | [null, null, null, null, null, 1]
+(1 row)
+
+-- create the whole path
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{}');
+update test_jsonb_subscript set test_json['a'][0]['b'][0]['c'] = '1';
+select * from test_jsonb_subscript;
+ id |         test_json          
+----+----------------------------
+  1 | {"a": [{"b": [{"c": 1}]}]}
+(1 row)
+
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{}');
+update test_jsonb_subscript set test_json['a'][2]['b'][2]['c'][2] = '1';
+select * from test_jsonb_subscript;
+ id |                            test_json                             
+----+------------------------------------------------------------------
+  1 | {"a": [null, null, {"b": [null, null, {"c": [null, null, 1]}]}]}
+(1 row)
+
+-- create the whole path with already existing keys
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{"b": 1}');
+update test_jsonb_subscript set test_json['a'][0] = '2';
+select * from test_jsonb_subscript;
+ id |     test_json      
+----+--------------------
+  1 | {"a": [2], "b": 1}
+(1 row)
+
+-- the start jsonb is an object, first subscript is treated as a key
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{}');
+update test_jsonb_subscript set test_json[0]['a'] = '1';
+select * from test_jsonb_subscript;
+ id |    test_json    
+----+-----------------
+  1 | {"0": {"a": 1}}
+(1 row)
+
+-- the start jsonb is an array
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[]');
+update test_jsonb_subscript set test_json[0]['a'] = '1';
+update test_jsonb_subscript set test_json[2]['b'] = '2';
+select * from test_jsonb_subscript;
+ id |         test_json          
+----+----------------------------
+  1 | [{"a": 1}, null, {"b": 2}]
+(1 row)
+
 -- jsonb to tsvector
 select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
                                 to_tsvector                                
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 20aa8fe0e2..812a92d37e 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1290,6 +1290,56 @@ update test_jsonb_subscript set test_json = NULL where id = 3;
 update test_jsonb_subscript set test_json[0] = '1';
 select * from test_jsonb_subscript;
 
+-- Fill the gaps logic
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[0]');
+
+update test_jsonb_subscript set test_json[5] = '1';
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json[-4] = '1';
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json[-8] = '1';
+select * from test_jsonb_subscript;
+
+-- keep consistent values position
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[]');
+
+update test_jsonb_subscript set test_json[5] = '1';
+select * from test_jsonb_subscript;
+
+-- create the whole path
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{}');
+update test_jsonb_subscript set test_json['a'][0]['b'][0]['c'] = '1';
+select * from test_jsonb_subscript;
+
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{}');
+update test_jsonb_subscript set test_json['a'][2]['b'][2]['c'][2] = '1';
+select * from test_jsonb_subscript;
+
+-- create the whole path with already existing keys
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{"b": 1}');
+update test_jsonb_subscript set test_json['a'][0] = '2';
+select * from test_jsonb_subscript;
+
+-- the start jsonb is an object, first subscript is treated as a key
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '{}');
+update test_jsonb_subscript set test_json[0]['a'] = '1';
+select * from test_jsonb_subscript;
+
+-- the start jsonb is an array
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[]');
+update test_jsonb_subscript set test_json[0]['a'] = '1';
+update test_jsonb_subscript set test_json[2]['b'] = '2';
+select * from test_jsonb_subscript;
+
 -- jsonb to tsvector
 select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
 
-- 
2.21.0


--bnxkghd4qzx6dtmd--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Should we document how column DEFAULT expressions work?
@ 2024-06-26 01:30 David G. Johnston <[email protected]>
  2024-06-26 04:50 ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: David G. Johnston @ 2024-06-26 01:30 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: James Coleman <[email protected]>; pgsql-hackers

On Tue, Jun 25, 2024 at 4:11 PM Tom Lane <[email protected]> wrote:

> James Coleman <[email protected]> writes:
> > On Tue, Jun 25, 2024 at 4:59 PM Tom Lane <[email protected]> wrote:
> >> Uh ... what?  I recall something about that with respect to certain
> >> features such as nextval(), but you're making it sound like there
> >> is something generic going on with DEFAULT.
>
> > Hmm, I guess I'd never considered anything besides cases like
> > nextval() and now(), but I see now that now() must also be special
> > cased (when quoted) since 'date_trunc(day, now())'::timestamp doesn't
> > work but 'now()'::timestamp does.
>
> Hmm, both of those behaviors are documented, but not in the same place
> and possibly not anywhere near where you looked for info about
> DEFAULT.  For instance, the Tip at the bottom of section 9.9.5
>
>
> https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-CURRENT
>
> explains about how 'now'::timestamp isn't what to use in DEFAULT.
>
>
I'd suggest adding to:

DEFAULT default_expr
The DEFAULT clause assigns a default data value for the column whose column
definition it appears within. The value is any variable-free expression (in
particular, cross-references to other columns in the current table are not
allowed). Subqueries are not allowed either. The data type of the default
expression must match the data type of the column.

The default expression will be used in any insert operation that does not
specify a value for the column. If there is no default for a column, then
the default is null.

+ Be aware that the [special timestamp values 1] are resolved immediately,
not upon insert.  Use the [date/time constructor functions 2] to produce a
time relative to the future insertion.

[1]
https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-SPECIAL-VALUES
[2]
https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-CURRENT

David J.


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Should we document how column DEFAULT expressions work?
  2024-06-26 01:30 Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
@ 2024-06-26 04:50 ` David Rowley <[email protected]>
  2024-06-26 05:12   ` Re: Should we document how column DEFAULT expressions work? Tom Lane <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: David Rowley @ 2024-06-26 04:50 UTC (permalink / raw)
  To: David G. Johnston <[email protected]>; +Cc: Tom Lane <[email protected]>; James Coleman <[email protected]>; pgsql-hackers

On Wed, 26 Jun 2024 at 13:31, David G. Johnston
<[email protected]> wrote:
> I'd suggest adding to:
>
> DEFAULT default_expr
> The DEFAULT clause assigns a default data value for the column whose column definition it appears within. The value is any variable-free expression (in particular, cross-references to other columns in the current table are not allowed). Subqueries are not allowed either. The data type of the default expression must match the data type of the column.
>
> The default expression will be used in any insert operation that does not specify a value for the column. If there is no default for a column, then the default is null.
>
> + Be aware that the [special timestamp values 1] are resolved immediately, not upon insert.  Use the [date/time constructor functions 2] to produce a time relative to the future insertion.

FWIW, I disagree that we need to write anything about that in this
part of the documentation.  I think any argument for doing this could
equally be applied to something like re-iterating what the operator
precedence rules for arithmetic are, and I don't think that should be
mentioned. Also, what about all the other places where someone could
use one of the special timestamp input values? Should CREATE VIEW get
a memo too?  How about PREPARE?

If people don't properly understand these special timestamp input
values, then maybe the documentation in [1] needs to be improved.  At
the moment the details are within parentheses. Namely "(In particular,
now and related strings are converted to a specific time value as soon
as they are read.)".  Maybe it would be better to be more explicit
there and mention that these are special values that the input
function understands which are translated to actual timestamp values
when the type's input function is called.  That could maybe be tied
into the DEFAULT clause documentation to mention that the input
function for constant values is called at DML time rather than DDL
time.  That way, we're not adding these (unsustainable) special cases
to the documentation.

David

[1] https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-SPECIAL-VALUES






^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Should we document how column DEFAULT expressions work?
  2024-06-26 01:30 Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
  2024-06-26 04:50 ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
@ 2024-06-26 05:12   ` Tom Lane <[email protected]>
  2024-06-26 06:00     ` Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
  2024-06-26 23:19     ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
  0 siblings, 2 replies; 7+ messages in thread

From: Tom Lane @ 2024-06-26 05:12 UTC (permalink / raw)
  To: David Rowley <[email protected]>; +Cc: David G. Johnston <[email protected]>; James Coleman <[email protected]>; pgsql-hackers

David Rowley <[email protected]> writes:
> If people don't properly understand these special timestamp input
> values, then maybe the documentation in [1] needs to be improved.  At
> the moment the details are within parentheses. Namely "(In particular,
> now and related strings are converted to a specific time value as soon
> as they are read.)".  Maybe it would be better to be more explicit
> there and mention that these are special values that the input
> function understands which are translated to actual timestamp values
> when the type's input function is called.  That could maybe be tied
> into the DEFAULT clause documentation to mention that the input
> function for constant values is called at DML time rather than DDL
> time.  That way, we're not adding these (unsustainable) special cases
> to the documentation.

This sounds like a reasonable approach to me for the
magic-input-values issue.  Do we want to do anything about
nextval()?  I guess if you hold your head at the correct
angle, that's also a magic-input-value issue, in the sense
that the question is when does regclass input get resolved.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Should we document how column DEFAULT expressions work?
  2024-06-26 01:30 Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
  2024-06-26 04:50 ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
  2024-06-26 05:12   ` Re: Should we document how column DEFAULT expressions work? Tom Lane <[email protected]>
@ 2024-06-26 06:00     ` David G. Johnston <[email protected]>
  1 sibling, 0 replies; 7+ messages in thread

From: David G. Johnston @ 2024-06-26 06:00 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: David Rowley <[email protected]>; James Coleman <[email protected]>; pgsql-hackers

On Tue, Jun 25, 2024 at 10:12 PM Tom Lane <[email protected]> wrote:

> David Rowley <[email protected]> writes:
> > If people don't properly understand these special timestamp input
> > values, then maybe the documentation in [1] needs to be improved.  At
> > the moment the details are within parentheses. Namely "(In particular,
> > now and related strings are converted to a specific time value as soon
> > as they are read.)".  Maybe it would be better to be more explicit
> > there and mention that these are special values that the input
> > function understands which are translated to actual timestamp values
> > when the type's input function is called.  That could maybe be tied
> > into the DEFAULT clause documentation to mention that the input
> > function for constant values is called at DML time rather than DDL
> > time.  That way, we're not adding these (unsustainable) special cases
> > to the documentation.
>
> This sounds like a reasonable approach to me for the
> magic-input-values issue.  Do we want to do anything about
> nextval()?  I guess if you hold your head at the correct
> angle, that's also a magic-input-value issue, in the sense
> that the question is when does regclass input get resolved.
>
>
From observations we transform constants into the: " 'value'::type " syntax
which then makes it an operator resolved at execution time.  For every type
except time types the transformation leaves the constant as-is.  The
special time values are the exception whereby they get evaluated to a
specific time during the transformation.

postgres=# create table tser3 (id integer not null default nextval(regclass
'tser2_id_seq'));
CREATE TABLE
postgres=# \d tser3
                            Table "public.tser3"
 Column |  Type   | Collation | Nullable |              Default

--------+---------+-----------+----------+-----------------------------------
 id     | integer |           | not null | nextval('tser2_id_seq'::regclass)

I cannot figure out how to get "early binding" into the default. I.e.,
nextval(9000)

Since early binding is similar to the special timestamp behavior I'd say
nextval is behaving just as expected - literal transform, no evaluation.
We need only document the transforms that also evaluate.

David J.


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Should we document how column DEFAULT expressions work?
  2024-06-26 01:30 Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
  2024-06-26 04:50 ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
  2024-06-26 05:12   ` Re: Should we document how column DEFAULT expressions work? Tom Lane <[email protected]>
@ 2024-06-26 23:19     ` David Rowley <[email protected]>
  2024-06-26 23:38       ` Re: Should we document how column DEFAULT expressions work? Tom Lane <[email protected]>
  1 sibling, 1 reply; 7+ messages in thread

From: David Rowley @ 2024-06-26 23:19 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: David G. Johnston <[email protected]>; James Coleman <[email protected]>; pgsql-hackers

On Wed, 26 Jun 2024 at 17:12, Tom Lane <[email protected]> wrote:
> Do we want to do anything about
> nextval()?  I guess if you hold your head at the correct
> angle, that's also a magic-input-value issue, in the sense
> that the question is when does regclass input get resolved.

I think I'm not understanding what's special about that.  Aren't
'now'::timestamp and 'seq_name'::regclass are just casts that are
evaluated during parse time in transformExpr()?

David






^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Should we document how column DEFAULT expressions work?
  2024-06-26 01:30 Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
  2024-06-26 04:50 ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
  2024-06-26 05:12   ` Re: Should we document how column DEFAULT expressions work? Tom Lane <[email protected]>
  2024-06-26 23:19     ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
@ 2024-06-26 23:38       ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Tom Lane @ 2024-06-26 23:38 UTC (permalink / raw)
  To: David Rowley <[email protected]>; +Cc: David G. Johnston <[email protected]>; James Coleman <[email protected]>; pgsql-hackers

David Rowley <[email protected]> writes:
> On Wed, 26 Jun 2024 at 17:12, Tom Lane <[email protected]> wrote:
>> Do we want to do anything about
>> nextval()?  I guess if you hold your head at the correct
>> angle, that's also a magic-input-value issue, in the sense
>> that the question is when does regclass input get resolved.

> I think I'm not understanding what's special about that.  Aren't
> 'now'::timestamp and 'seq_name'::regclass are just casts that are
> evaluated during parse time in transformExpr()?

Right.  But there is an example in the manual explaining how
these two things act differently:

	'seq_name'::regclass
	'seq_name'::text::regclass

The latter produces a constant of type text with a run-time
cast to regclass (and hence a run-time pg_class lookup).
IIRC, we document that mainly because the latter provides a way
to duplicate nextval()'s old behavior of run-time lookup.

Now that I think about it, there's a very parallel difference in
the behavior of

	'now'::timestamp
	'now'::text::timestamp

but I doubt that that example is shown anywhere.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2024-06-26 23:38 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-31 14:19 [PATCH v41 2/2] Filling gaps in jsonb Dmitrii Dolgov <[email protected]>
2024-06-26 01:30 Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
2024-06-26 04:50 ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
2024-06-26 05:12   ` Re: Should we document how column DEFAULT expressions work? Tom Lane <[email protected]>
2024-06-26 06:00     ` Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]>
2024-06-26 23:19     ` Re: Should we document how column DEFAULT expressions work? David Rowley <[email protected]>
2024-06-26 23:38       ` Re: Should we document how column DEFAULT expressions work? Tom Lane <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox