public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v8 1/7] Row pattern recognition patch for raw parser.
15+ messages / 6 participants
[nested] [flat]

* [PATCH v8 1/7] Row pattern recognition patch for raw parser.
@ 2023-09-25 05:01 Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 15+ messages in thread

From: Tatsuo Ishii @ 2023-09-25 05:01 UTC (permalink / raw)

---
 src/backend/parser/gram.y       | 222 ++++++++++++++++++++++++++++++--
 src/include/nodes/parsenodes.h  |  56 ++++++++
 src/include/parser/kwlist.h     |   8 ++
 src/include/parser/parse_node.h |   1 +
 4 files changed, 273 insertions(+), 14 deletions(-)

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7d2032885e..74c2069050 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -251,6 +251,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	DefElem	   *defelt;
 	SortBy	   *sortby;
 	WindowDef  *windef;
+	RPCommonSyntax	*rpcom;
+	RPSubsetItem	*rpsubset;
 	JoinExpr   *jexpr;
 	IndexElem  *ielem;
 	StatsElem  *selem;
@@ -278,6 +280,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	MergeWhenClause *mergewhen;
 	struct KeyActions *keyactions;
 	struct KeyAction *keyaction;
+	RPSkipTo	skipto;
 }
 
 %type <node>	stmt toplevel_stmt schema_stmt routine_body_stmt
@@ -453,8 +456,12 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 				TriggerTransitions TriggerReferencing
 				vacuum_relation_list opt_vacuum_relation_list
 				drop_option_list pub_obj_list
-
-%type <node>	opt_routine_body
+				row_pattern_measure_list row_pattern_definition_list
+				opt_row_pattern_subset_clause
+				row_pattern_subset_list row_pattern_subset_rhs
+				row_pattern
+%type <rpsubset>	 row_pattern_subset_item
+%type <node>	opt_routine_body row_pattern_term
 %type <groupclause> group_clause
 %type <list>	group_by_list
 %type <node>	group_by_item empty_grouping_set rollup_clause cube_clause
@@ -551,6 +558,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <range>	relation_expr_opt_alias
 %type <node>	tablesample_clause opt_repeatable_clause
 %type <target>	target_el set_target insert_column_item
+				row_pattern_measure_item row_pattern_definition
+%type <skipto>	first_or_last
 
 %type <str>		generic_option_name
 %type <node>	generic_option_arg
@@ -633,6 +642,9 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>	window_clause window_definition_list opt_partition_clause
 %type <windef>	window_definition over_clause window_specification
 				opt_frame_clause frame_extent frame_bound
+%type <rpcom>	opt_row_pattern_common_syntax opt_row_pattern_skip_to
+%type <boolean>	opt_row_pattern_initial_or_seek
+%type <list>	opt_row_pattern_measures
 %type <ival>	opt_window_exclusion_clause
 %type <str>		opt_existing_window_name
 %type <boolean> opt_if_not_exists
@@ -659,7 +671,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 				json_object_constructor_null_clause_opt
 				json_array_constructor_null_clause_opt
 
-
 /*
  * Non-keyword token types.  These are hard-wired into the "flex" lexer.
  * They must be listed first so that their numeric codes do not depend on
@@ -702,7 +713,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	CURRENT_TIME CURRENT_TIMESTAMP CURRENT_USER CURSOR CYCLE
 
 	DATA_P DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
-	DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS DEPENDS DEPTH DESC
+	DEFERRABLE DEFERRED DEFINE DEFINER DELETE_P DELIMITER DELIMITERS DEPENDS DEPTH DESC
 	DETACH DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P
 	DOUBLE_P DROP
 
@@ -718,7 +729,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	HANDLER HAVING HEADER_P HOLD HOUR_P
 
 	IDENTITY_P IF_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IMPORT_P IN_P INCLUDE
-	INCLUDING INCREMENT INDENT INDEX INDEXES INHERIT INHERITS INITIALLY INLINE_P
+	INCLUDING INCREMENT INDENT INDEX INDEXES INHERIT INHERITS INITIAL INITIALLY INLINE_P
 	INNER_P INOUT INPUT_P INSENSITIVE INSERT INSTEAD INT_P INTEGER
 	INTERSECT INTERVAL INTO INVOKER IS ISNULL ISOLATION
 
@@ -731,7 +742,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
 	LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P LOCKED LOGGED
 
-	MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MERGE METHOD
+	MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MEASURES MERGE METHOD
 	MINUTE_P MINVALUE MODE MONTH_P MOVE
 
 	NAME_P NAMES NATIONAL NATURAL NCHAR NEW NEXT NFC NFD NFKC NFKD NO NONE
@@ -743,8 +754,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	ORDER ORDINALITY OTHERS OUT_P OUTER_P
 	OVER OVERLAPS OVERLAY OVERRIDING OWNED OWNER
 
-	PARALLEL PARAMETER PARSER PARTIAL PARTITION PASSING PASSWORD
-	PLACING PLANS POLICY
+	PARALLEL PARAMETER PARSER PARTIAL PARTITION PASSING PASSWORD PAST
+	PATTERN_P PERMUTE PLACING PLANS POLICY
 	POSITION PRECEDING PRECISION PRESERVE PREPARE PREPARED PRIMARY
 	PRIOR PRIVILEGES PROCEDURAL PROCEDURE PROCEDURES PROGRAM PUBLICATION
 
@@ -755,12 +766,13 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	RESET RESTART RESTRICT RETURN RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP
 	ROUTINE ROUTINES ROW ROWS RULE
 
-	SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT
+	SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SEEK SELECT
 	SEQUENCE SEQUENCES
+
 	SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW
 	SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P
 	START STATEMENT STATISTICS STDIN STDOUT STORAGE STORED STRICT_P STRIP_P
-	SUBSCRIPTION SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER
+	SUBSCRIPTION SUBSET SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER
 
 	TABLE TABLES TABLESAMPLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN
 	TIES TIME TIMESTAMP TO TRAILING TRANSACTION TRANSFORM
@@ -853,6 +865,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
  */
 %nonassoc	UNBOUNDED		/* ideally would have same precedence as IDENT */
 %nonassoc	IDENT PARTITION RANGE ROWS GROUPS PRECEDING FOLLOWING CUBE ROLLUP
+%nonassoc	MEASURES AFTER INITIAL SEEK PATTERN_P
 %left		Op OPERATOR		/* multi-character ops and user-defined operators */
 %left		'+' '-'
 %left		'*' '/' '%'
@@ -15857,7 +15870,8 @@ over_clause: OVER window_specification
 		;
 
 window_specification: '(' opt_existing_window_name opt_partition_clause
-						opt_sort_clause opt_frame_clause ')'
+						opt_sort_clause opt_row_pattern_measures opt_frame_clause
+						opt_row_pattern_common_syntax ')'
 				{
 					WindowDef  *n = makeNode(WindowDef);
 
@@ -15865,10 +15879,12 @@ window_specification: '(' opt_existing_window_name opt_partition_clause
 					n->refname = $2;
 					n->partitionClause = $3;
 					n->orderClause = $4;
+					n->rowPatternMeasures = $5;
 					/* copy relevant fields of opt_frame_clause */
-					n->frameOptions = $5->frameOptions;
-					n->startOffset = $5->startOffset;
-					n->endOffset = $5->endOffset;
+					n->frameOptions = $6->frameOptions;
+					n->startOffset = $6->startOffset;
+					n->endOffset = $6->endOffset;
+					n->rpCommonSyntax = $7;
 					n->location = @1;
 					$$ = n;
 				}
@@ -15892,6 +15908,31 @@ opt_partition_clause: PARTITION BY expr_list		{ $$ = $3; }
 			| /*EMPTY*/								{ $$ = NIL; }
 		;
 
+/*
+ * ROW PATTERN_P MEASURES
+ */
+opt_row_pattern_measures: MEASURES row_pattern_measure_list	{ $$ = $2; }
+			| /*EMPTY*/								{ $$ = NIL; }
+		;
+
+row_pattern_measure_list:
+			row_pattern_measure_item
+					{ $$ = list_make1($1); }
+			| row_pattern_measure_list ',' row_pattern_measure_item
+					{ $$ = lappend($1, $3); }
+		;
+
+row_pattern_measure_item:
+			a_expr AS ColLabel
+				{
+					$$ = makeNode(ResTarget);
+					$$->name = $3;
+					$$->indirection = NIL;
+					$$->val = (Node *) $1;
+					$$->location = @1;
+				}
+		;
+
 /*
  * For frame clauses, we return a WindowDef, but only some fields are used:
  * frameOptions, startOffset, and endOffset.
@@ -16051,6 +16092,143 @@ opt_window_exclusion_clause:
 			| /*EMPTY*/				{ $$ = 0; }
 		;
 
+opt_row_pattern_common_syntax:
+opt_row_pattern_skip_to opt_row_pattern_initial_or_seek
+				PATTERN_P '(' row_pattern ')'
+				opt_row_pattern_subset_clause
+				DEFINE row_pattern_definition_list
+			{
+				RPCommonSyntax *n = makeNode(RPCommonSyntax);
+				n->rpSkipTo = $1->rpSkipTo;
+				n->rpSkipVariable = $1->rpSkipVariable;
+				n->initial = $2;
+				n->rpPatterns = $5;
+				n->rpSubsetClause = $7;
+				n->rpDefs = $9;
+				$$ = n;
+			}
+			| /*EMPTY*/		{ $$ = NULL; }
+	;
+
+opt_row_pattern_skip_to:
+			AFTER MATCH SKIP TO NEXT ROW
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_NEXT_ROW;
+					n->rpSkipVariable = NULL;
+					$$ = n;
+			}
+			| AFTER MATCH SKIP PAST LAST_P ROW
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_PAST_LAST_ROW;
+					n->rpSkipVariable = NULL;
+					$$ = n;
+				}
+			| AFTER MATCH SKIP TO first_or_last ColId
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = $5;
+					n->rpSkipVariable = $6;
+					$$ = n;
+				}
+/*
+			| AFTER MATCH SKIP TO LAST_P ColId		%prec LAST_P
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_LAST_VARIABLE;
+					n->rpSkipVariable = $6;
+					$$ = n;
+				}
+			| AFTER MATCH SKIP TO ColId
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_VARIABLE;
+					n->rpSkipVariable = $5;
+					$$ = n;
+				}
+*/
+			| /*EMPTY*/
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					/* temporary set default to ST_NEXT_ROW */
+					n->rpSkipTo = ST_PAST_LAST_ROW;
+					n->rpSkipVariable = NULL;
+					$$ = n;
+				}
+	;
+
+first_or_last:
+			FIRST_P		{ $$ = ST_FIRST_VARIABLE; }
+			| LAST_P	{ $$ = ST_LAST_VARIABLE; }
+	;
+
+opt_row_pattern_initial_or_seek:
+			INITIAL			{ $$ = true; }
+			| SEEK
+				{
+					ereport(ERROR,
+							(errcode(ERRCODE_SYNTAX_ERROR),
+							 errmsg("SEEK is not supported"),
+							 errhint("Use INITIAL."),
+							 parser_errposition(@1)));
+				}
+			| /*EMPTY*/		{ $$ = true; }
+		;
+
+row_pattern:
+			row_pattern_term							{ $$ = list_make1($1); }
+			| row_pattern row_pattern_term				{ $$ = lappend($1, $2); }
+		;
+
+row_pattern_term:
+			ColId	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "", (Node *)makeString($1), NULL, @1); }
+			| ColId '*'	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "*", (Node *)makeString($1), NULL, @1); }
+			| ColId '+'	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "+", (Node *)makeString($1), NULL, @1); }
+			| ColId '?'	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "?", (Node *)makeString($1), NULL, @1); }
+		;
+
+opt_row_pattern_subset_clause:
+			SUBSET row_pattern_subset_list	{ $$ = $2; }
+			| /*EMPTY*/												{ $$ = NIL; }
+		;
+
+row_pattern_subset_list:
+			row_pattern_subset_item									{ $$ = list_make1($1); }
+			| row_pattern_subset_list ',' row_pattern_subset_item	{ $$ = lappend($1, $3); }
+			| /*EMPTY*/												{ $$ = NIL; }
+		;
+
+row_pattern_subset_item: ColId '=' '(' row_pattern_subset_rhs ')'
+			{
+				RPSubsetItem *n = makeNode(RPSubsetItem);
+				n->name = $1;
+				n->rhsVariable = $4;
+				$$ = n;
+			}
+		;
+
+row_pattern_subset_rhs:
+			ColId								{ $$ = list_make1(makeStringConst($1, @1)); }
+			| row_pattern_subset_rhs ',' ColId	{ $$ = lappend($1, makeStringConst($3, @1)); }
+			| /*EMPTY*/							{ $$ = NIL; }
+		;
+
+row_pattern_definition_list:
+			row_pattern_definition										{ $$ = list_make1($1); }
+			| row_pattern_definition_list ',' row_pattern_definition	{ $$ = lappend($1, $3); }
+		;
+
+row_pattern_definition:
+			ColId AS a_expr
+				{
+					$$ = makeNode(ResTarget);
+					$$->name = $1;
+					$$->indirection = NIL;
+					$$->val = (Node *) $3;
+					$$->location = @1;
+				}
+		;
 
 /*
  * Supporting nonterminals for expressions.
@@ -17146,6 +17324,7 @@ unreserved_keyword:
 			| INDEXES
 			| INHERIT
 			| INHERITS
+			| INITIAL
 			| INLINE_P
 			| INPUT_P
 			| INSENSITIVE
@@ -17173,6 +17352,7 @@ unreserved_keyword:
 			| MATCHED
 			| MATERIALIZED
 			| MAXVALUE
+			| MEASURES
 			| MERGE
 			| METHOD
 			| MINUTE_P
@@ -17215,6 +17395,9 @@ unreserved_keyword:
 			| PARTITION
 			| PASSING
 			| PASSWORD
+			| PAST
+			| PATTERN_P
+			| PERMUTE
 			| PLANS
 			| POLICY
 			| PRECEDING
@@ -17265,6 +17448,7 @@ unreserved_keyword:
 			| SEARCH
 			| SECOND_P
 			| SECURITY
+			| SEEK
 			| SEQUENCE
 			| SEQUENCES
 			| SERIALIZABLE
@@ -17290,6 +17474,7 @@ unreserved_keyword:
 			| STRICT_P
 			| STRIP_P
 			| SUBSCRIPTION
+			| SUBSET
 			| SUPPORT
 			| SYSID
 			| SYSTEM_P
@@ -17477,6 +17662,7 @@ reserved_keyword:
 			| CURRENT_USER
 			| DEFAULT
 			| DEFERRABLE
+			| DEFINE
 			| DESC
 			| DISTINCT
 			| DO
@@ -17639,6 +17825,7 @@ bare_label_keyword:
 			| DEFAULTS
 			| DEFERRABLE
 			| DEFERRED
+			| DEFINE
 			| DEFINER
 			| DELETE_P
 			| DELIMITER
@@ -17714,6 +17901,7 @@ bare_label_keyword:
 			| INDEXES
 			| INHERIT
 			| INHERITS
+			| INITIAL
 			| INITIALLY
 			| INLINE_P
 			| INNER_P
@@ -17763,6 +17951,7 @@ bare_label_keyword:
 			| MATCHED
 			| MATERIALIZED
 			| MAXVALUE
+			| MEASURES
 			| MERGE
 			| METHOD
 			| MINVALUE
@@ -17816,6 +18005,9 @@ bare_label_keyword:
 			| PARTITION
 			| PASSING
 			| PASSWORD
+			| PAST
+			| PATTERN_P
+			| PERMUTE
 			| PLACING
 			| PLANS
 			| POLICY
@@ -17872,6 +18064,7 @@ bare_label_keyword:
 			| SCROLL
 			| SEARCH
 			| SECURITY
+			| SEEK
 			| SELECT
 			| SEQUENCE
 			| SEQUENCES
@@ -17903,6 +18096,7 @@ bare_label_keyword:
 			| STRICT_P
 			| STRIP_P
 			| SUBSCRIPTION
+			| SUBSET
 			| SUBSTRING
 			| SUPPORT
 			| SYMMETRIC
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index fef4c714b8..657651df1d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -547,6 +547,44 @@ typedef struct SortBy
 	int			location;		/* operator location, or -1 if none/unknown */
 } SortBy;
 
+/*
+ * AFTER MATCH row pattern skip to types in row pattern common syntax
+ */
+typedef enum RPSkipTo
+{
+	ST_NONE,					/* AFTER MATCH omitted */
+	ST_NEXT_ROW,				/* SKIP TO NEXT ROW */
+	ST_PAST_LAST_ROW,			/* SKIP TO PAST LAST ROW */
+	ST_FIRST_VARIABLE,			/* SKIP TO FIRST variable name */
+	ST_LAST_VARIABLE,			/* SKIP TO LAST variable name */
+	ST_VARIABLE					/* SKIP TO variable name */
+} RPSkipTo;
+
+/*
+ * Row Pattern SUBSET clause item
+ */
+typedef struct RPSubsetItem
+{
+	NodeTag		type;
+	char	   *name;			/* Row Pattern SUBSET clause variable name */
+	List	   *rhsVariable;	/* Row Pattern SUBSET rhs variables (list of char *string) */
+} RPSubsetItem;
+
+/*
+ * RowPatternCommonSyntax - raw representation of row pattern common syntax
+ *
+ */
+typedef struct RPCommonSyntax
+{
+	NodeTag		type;
+	RPSkipTo	rpSkipTo;		/* Row Pattern AFTER MATCH SKIP type */
+	char	   *rpSkipVariable;	/* Row Pattern Skip To variable name, if any */
+	bool		initial;		/* true if <row pattern initial or seek> is initial */
+	List	   *rpPatterns;		/* PATTERN variables (list of A_Expr) */
+	List	   *rpSubsetClause;	/* row pattern subset clause (list of RPSubsetItem), if any */
+	List	   *rpDefs;			/* row pattern definitions clause (list of ResTarget) */
+} RPCommonSyntax;
+
 /*
  * WindowDef - raw representation of WINDOW and OVER clauses
  *
@@ -562,6 +600,8 @@ typedef struct WindowDef
 	char	   *refname;		/* referenced window name, if any */
 	List	   *partitionClause;	/* PARTITION BY expression list */
 	List	   *orderClause;	/* ORDER BY (list of SortBy) */
+	List	   *rowPatternMeasures;	/* row pattern measures (list of ResTarget) */
+	RPCommonSyntax *rpCommonSyntax;	/* row pattern common syntax */
 	int			frameOptions;	/* frame_clause options, see below */
 	Node	   *startOffset;	/* expression for starting bound, if any */
 	Node	   *endOffset;		/* expression for ending bound, if any */
@@ -1483,6 +1523,11 @@ typedef struct GroupingSet
  * the orderClause might or might not be copied (see copiedOrder); the framing
  * options are never copied, per spec.
  *
+ * "defineClause" is Row Pattern Recognition DEFINE clause (list of
+ * TargetEntry). TargetEntry.resname represents row pattern definition
+ * variable name. "patternVariable" and "patternRegexp" represents PATTERN
+ * clause.
+ *
  * The information relevant for the query jumbling is the partition clause
  * type and its bounds.
  */
@@ -1514,6 +1559,17 @@ typedef struct WindowClause
 	Index		winref;			/* ID referenced by window functions */
 	/* did we copy orderClause from refname? */
 	bool		copiedOrder pg_node_attr(query_jumble_ignore);
+	/* Row Pattern AFTER MACH SKIP clause */
+	RPSkipTo	rpSkipTo;		/* Row Pattern Skip To type */
+	bool		initial;		/* true if <row pattern initial or seek> is initial */
+	/* Row Pattern DEFINE clause (list of TargetEntry) */
+	List		*defineClause;
+	/* Row Pattern DEFINE variable initial names (list of String) */
+	List		*defineInitial;
+	/* Row Pattern PATTERN variable name (list of String) */
+	List		*patternVariable;
+	/* Row Pattern PATTERN regular expression quantifier ('+' or ''. list of String) */
+	List		*patternRegexp;
 } WindowClause;
 
 /*
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 5984dcfa4b..2804333b53 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -128,6 +128,7 @@ PG_KEYWORD("default", DEFAULT, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("defaults", DEFAULTS, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("deferrable", DEFERRABLE, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("deferred", DEFERRED, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("define", DEFINE, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("definer", DEFINER, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("delete", DELETE_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("delimiter", DELIMITER, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -212,6 +213,7 @@ PG_KEYWORD("index", INDEX, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("indexes", INDEXES, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inherit", INHERIT, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inherits", INHERITS, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("initial", INITIAL, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("initially", INITIALLY, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inline", INLINE_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inner", INNER_P, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
@@ -265,6 +267,7 @@ PG_KEYWORD("match", MATCH, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("matched", MATCHED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("materialized", MATERIALIZED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("maxvalue", MAXVALUE, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("measures", MEASURES, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("merge", MERGE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("method", METHOD, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("minute", MINUTE_P, UNRESERVED_KEYWORD, AS_LABEL)
@@ -326,6 +329,9 @@ PG_KEYWORD("partial", PARTIAL, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("partition", PARTITION, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("passing", PASSING, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("password", PASSWORD, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("past", PAST, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("pattern", PATTERN_P, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("permute", PERMUTE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("placing", PLACING, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("plans", PLANS, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("policy", POLICY, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -385,6 +391,7 @@ PG_KEYWORD("scroll", SCROLL, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("search", SEARCH, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("second", SECOND_P, UNRESERVED_KEYWORD, AS_LABEL)
 PG_KEYWORD("security", SECURITY, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("seek", SEEK, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("select", SELECT, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("sequence", SEQUENCE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("sequences", SEQUENCES, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -416,6 +423,7 @@ PG_KEYWORD("stored", STORED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("strict", STRICT_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("strip", STRIP_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("subscription", SUBSCRIPTION, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("subset", SUBSET, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("substring", SUBSTRING, COL_NAME_KEYWORD, BARE_LABEL)
 PG_KEYWORD("support", SUPPORT, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("symmetric", SYMMETRIC, RESERVED_KEYWORD, BARE_LABEL)
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index f589112d5e..6640090910 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -51,6 +51,7 @@ typedef enum ParseExprKind
 	EXPR_KIND_WINDOW_FRAME_RANGE,	/* window frame clause with RANGE */
 	EXPR_KIND_WINDOW_FRAME_ROWS,	/* window frame clause with ROWS */
 	EXPR_KIND_WINDOW_FRAME_GROUPS,	/* window frame clause with GROUPS */
+	EXPR_KIND_RPR_DEFINE,		/* DEFINE */
 	EXPR_KIND_SELECT_TARGET,	/* SELECT target list item */
 	EXPR_KIND_INSERT_TARGET,	/* INSERT target list item */
 	EXPR_KIND_UPDATE_SOURCE,	/* UPDATE assignment source item */
-- 
2.25.1


----Next_Part(Mon_Sep_25_14_26_30_2023_752)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v8-0002-Row-pattern-recognition-patch-parse-analysis.patch"



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

* Re: Remove dependence on integer wrapping
@ 2024-08-09 01:01 jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: jian he @ 2024-08-09 01:01 UTC (permalink / raw)
  To: Matthew Kim <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; Nathan Bossart <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Fri, Aug 9, 2024 at 6:16 AM Matthew Kim <[email protected]> wrote:
>
> I've updated patch 0004 to check the return value of pg_mul_s32_overflow(). Since tm.tm_year overflowed, the error message is hardcoded.
>

--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,10 @@ make_date(PG_FUNCTION_ARGS)
  if (tm.tm_year < 0)
  {
  bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_mul_s32_overflow(tm.tm_year, -1, &tm.tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date field value out of range")));
  }

Should the error about integers be out of range?

SELECT make_date(-2147483648, 1, 1);
"-2147483648" is not an allowed integer.


\df make_date
                                      List of functions
   Schema   |   Name    | Result data type |           Argument data
types            | Type
------------+-----------+------------------+------------------------------------------+------
 pg_catalog | make_date | date             | year integer, month
integer, day integer | func






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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
@ 2024-08-10 15:41 ` Joseph Koshakow <[email protected]>
  2024-08-10 21:54   ` Re: Remove dependence on integer wrapping Matthew Kim <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  0 siblings, 2 replies; 15+ messages in thread

From: Joseph Koshakow @ 2024-08-10 15:41 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Matthew Kim <[email protected]>; Nathan Bossart <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Thu, Aug 8, 2024 at 9:01 PM jian he <[email protected]> wrote:
>
> Should the error about integers be out of range?
>
> SELECT make_date(-2147483648, 1, 1);
> "-2147483648" is not an allowed integer.
>
> \df make_date
>                                       List of functions
>    Schema   |   Name    | Result data type |           Argument data
> types            | Type
>
------------+-----------+------------------+------------------------------------------+------
>  pg_catalog | make_date | date             | year integer, month
> integer, day integer | func

Are you saying that with the patch applied you're seeing the above
error? If so, I see a different error.

    test=# SELECT make_date(-2147483648, 1, 1);
    ERROR:  date field value out of range

Or are you saying that we should change the code in the patch so that
it returns the above error? If so, I'm not sure I understand the
reasoning. -2147483648 is an allowed integer, it's the minimum allowed
value for integers.

    test=# SELECT (-2147483648)::integer;
        int4
    -------------
     -2147483648
    (1 row)

Thanks,
Joseph Koshakow


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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
@ 2024-08-10 21:54   ` Matthew Kim <[email protected]>
  1 sibling, 0 replies; 15+ messages in thread

From: Matthew Kim @ 2024-08-10 21:54 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: jian he <[email protected]>; Nathan Bossart <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Mon, Jul 22, 2024 at 5:52 PM Alexander Lakhin
<exclusion(at)gmail(dot)com>
wrote:

> Also there are several trap-producing cases with date types:
> SELECT to_timestamp('1000000000,999', 'Y,YYY');

I attached patch 5 that fixes the to_timestamp overflow.

Thank you,
Matthew Kim


Attachments:

  [application/x-patch] v18-0003-Handle-overflows-in-do_to_timestamp.patch (3.1K, ../../CAGzNdfFs4hN=_2SZzZivoNtR8H1DiYHc+Zgh_oLv16EgXLuZGg@mail.gmail.com/3-v18-0003-Handle-overflows-in-do_to_timestamp.patch)
  download | inline diff:
From a01c5d67894de89f14f0dfc54d32e92258a1a3a7 Mon Sep 17 00:00:00 2001
From: Matthew Kim <[email protected]>
Date: Tue, 9 Jul 2024 18:25:10 -0400
Subject: [PATCH 3/5] Handle overflows in do_to_timestamp().

This commit handles overflow when formatting timestamps with the 'CC'
pattern.
---
 src/backend/utils/adt/formatting.c     | 25 +++++++++++++++++++++++--
 src/test/regress/expected/horology.out |  2 ++
 src/test/regress/sql/horology.sql      |  1 +
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68069fcfd3..decf0b6123 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
 
 #include "catalog/pg_collation.h"
 #include "catalog/pg_type.h"
+#include "common/int.h"
 #include "common/unicode_case.h"
 #include "common/unicode_category.h"
 #include "mb/pg_wchar.h"
@@ -4797,11 +4798,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
 		if (tmfc.bc)
 			tmfc.cc = -tmfc.cc;
 		if (tmfc.cc >= 0)
+		{
 			/* +1 because 21st century started in 2001 */
-			tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+			/* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+			if (pg_mul_s32_overflow((tmfc.cc - 1), 100, &tm->tm_year) ||
+				pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+						 errmsg("date out of range: \"%s\"",
+								text_to_cstring(date_txt))));
+			}
+		}
 		else
+		{
 			/* +1 because year == 599 is 600 BC */
-			tm->tm_year = tmfc.cc * 100 + 1;
+			/* tm->tm_year = tmfc.cc * 100 + 1; */
+			if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+				pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+						 errmsg("date out of range: \"%s\"",
+								text_to_cstring(date_txt))));
+			}
+		}
 		fmask |= DTK_M(YEAR);
 	}
 
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 241713cc51..311c688f89 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3778,6 +3778,8 @@ SELECT to_date('0000-02-01','YYYY-MM-DD');  -- allowed, though it shouldn't be
  02-01-0001 BC
 (1 row)
 
+SELECT to_date('100000000', 'CC');
+ERROR:  date out of range: "100000000"
 -- to_char's TZ format code produces zone abbrev if known
 SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
          to_char         
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index e5cf12ff63..12a035cf57 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -660,6 +660,7 @@ SELECT to_date('2016 365', 'YYYY DDD');  -- ok
 SELECT to_date('2016 366', 'YYYY DDD');  -- ok
 SELECT to_date('2016 367', 'YYYY DDD');
 SELECT to_date('0000-02-01','YYYY-MM-DD');  -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
 
 -- to_char's TZ format code produces zone abbrev if known
 SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
-- 
2.39.3 (Apple Git-146)



  [application/x-patch] v18-0004-Handle-overflow-when-taking-absolute-value-of-BC-yea.patch (1.9K, ../../CAGzNdfFs4hN=_2SZzZivoNtR8H1DiYHc+Zgh_oLv16EgXLuZGg@mail.gmail.com/4-v18-0004-Handle-overflow-when-taking-absolute-value-of-BC-yea.patch)
  download | inline diff:
From e22eb524a85dd3f2a363b61f5d91bfe34b0470d2 Mon Sep 17 00:00:00 2001
From: Matthew Kim <[email protected]>
Date: Wed, 7 Aug 2024 18:58:48 -0700
Subject: [PATCH 4/5] Handle overflow when taking absolute value of BC years

make_date formats negative years as BC years by taking the absolute value. This commit safely takes the absolute value of tm.tm_year
---
 src/backend/utils/adt/date.c       | 5 ++++-
 src/test/regress/expected/date.out | 2 ++
 src/test/regress/sql/date.sql      | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9c854e0e5c..baa677fb96 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,10 @@ make_date(PG_FUNCTION_ARGS)
 	if (tm.tm_year < 0)
 	{
 		bc = true;
-		tm.tm_year = -tm.tm_year;
+		if (pg_mul_s32_overflow(tm.tm_year, -1, &tm.tm_year))
+			ereport(ERROR,
+					(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+					 errmsg("date field value out of range")));
 	}
 
 	dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index f5949f3d17..9293e045b0 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1532,3 +1532,5 @@ select make_time(10, 55, 100.1);
 ERROR:  time field value out of range: 10:55:100.1
 select make_time(24, 0, 2.1);
 ERROR:  time field value out of range: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR:  date field value out of range
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make_date(2013, 13, 1);
 select make_date(2013, 11, -1);
 select make_time(10, 55, 100.1);
 select make_time(24, 0, 2.1);
+SELECT make_date(-2147483648, 1, 1);
-- 
2.39.3 (Apple Git-146)



  [application/x-patch] v18-0002-Remove-dependence-on-integer-wrapping-for-jsonb.patch (2.7K, ../../CAGzNdfFs4hN=_2SZzZivoNtR8H1DiYHc+Zgh_oLv16EgXLuZGg@mail.gmail.com/5-v18-0002-Remove-dependence-on-integer-wrapping-for-jsonb.patch)
  download | inline diff:
From 754e2cf9cb134d8756433de4c25ccb94468dbb7b Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 6 Jul 2024 15:41:09 -0400
Subject: [PATCH 2/5] Remove dependence on integer wrapping for jsonb

This commit updates various jsonb operators and functions to no longer
rely on integer wrapping for correctness. Not all compilers support
-fwrapv, so it's best not to rely on it.
---
 src/backend/utils/adt/jsonfuncs.c   |  6 +++---
 src/test/regress/expected/jsonb.out | 12 ++++++++++++
 src/test/regress/sql/jsonb.sql      |  2 ++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 5ecb9fffae..186c7e7dd8 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -946,7 +946,7 @@ jsonb_array_element(PG_FUNCTION_ARGS)
 	{
 		uint32		nelements = JB_ROOT_COUNT(jb);
 
-		if (-element > nelements)
+		if (element == PG_INT32_MIN || -element > nelements)
 			PG_RETURN_NULL();
 		else
 			element += nelements;
@@ -5426,7 +5426,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 
 	if (idx < 0)
 	{
-		if (-idx > nelems)
+		if (idx == PG_INT32_MIN || -idx > nelems)
 		{
 			/*
 			 * If asked to keep elements position consistent, it's not allowed
@@ -5438,7 +5438,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 						 errmsg("path element at position %d is out of range: %d",
 								level + 1, idx)));
 			else
-				idx = INT_MIN;
+				idx = PG_INT32_MIN;
 		}
 		else
 			idx = nelems + idx;
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index e66d760189..a9d93052fc 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -680,6 +680,18 @@ select '"foo"'::jsonb -> 'z';
  
 (1 row)
 
+select '[]'::jsonb -> -2147483648;
+ ?column? 
+----------
+ 
+(1 row)
+
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
+ jsonb_delete_path 
+-------------------
+ {"a": []}
+(1 row)
+
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
  ?column? 
 ----------
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 97bc2242a1..6a18577ead 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -204,6 +204,8 @@ select '[{"b": "c"}, {"b": "cc"}]'::jsonb -> 'z';
 select '{"a": "c", "b": null}'::jsonb -> 'b';
 select '"foo"'::jsonb -> 1;
 select '"foo"'::jsonb -> 'z';
+select '[]'::jsonb -> -2147483648;
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
 
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::int;
-- 
2.39.3 (Apple Git-146)



  [application/x-patch] v18-0001-Remove-dependence-on-integer-wrapping.patch (14.4K, ../../CAGzNdfFs4hN=_2SZzZivoNtR8H1DiYHc+Zgh_oLv16EgXLuZGg@mail.gmail.com/6-v18-0001-Remove-dependence-on-integer-wrapping.patch)
  download | inline diff:
From 3b1b8cb216bdc778a7cd05daa9b77c2a024b00f2 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH 1/5] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               | 14 ++--
 src/backend/utils/adt/numeric.c            |  5 +-
 src/backend/utils/adt/numutils.c           | 34 ++++-----
 src/backend/utils/adt/timestamp.c          | 28 +------
 src/include/common/int.h                   | 86 ++++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +--
 src/test/regress/expected/timestamp.out    | 13 ++++
 src/test/regress/expected/timestamptz.out  | 13 ++++
 src/test/regress/sql/timestamp.sql         |  4 +
 src/test/regress/sql/timestamptz.sql       |  4 +
 10 files changed, 154 insertions(+), 58 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..4b67d930b3 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,11 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/*
+	 * make the amount positive for digit-reconstruction loop
+	 */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +474,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = (uvalue) / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d0f0923710..38965b4023 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8114,15 +8114,14 @@ int64_to_numericvar(int64 val, NumericVar *var)
 
 	/* int64 can require at most 19 decimal digits; add one for safety */
 	alloc_var(var, 20 / DEC_DIGITS);
+	uval = pg_abs_s64(val);
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
 	}
 	else
 	{
 		var->sign = NUMERIC_POS;
-		uval = val;
 	}
 	var->dscale = 0;
 	if (val == 0)
@@ -11443,7 +11442,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..a3d7d6bf01 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..c76793f72d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+		pg_add_s64_overflow(result, time, &result))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..04e0a8f3be 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,9 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true and leave *result unchanged, otherwise
+ * store the result of -a into *result.
+ * - Return the absolute value of a as an unsigned integer of the same width.
  *---------
  */
 
@@ -154,6 +157,12 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	return a < 0 ? 0 - (uint32) a : (uint32) a;
+}
+
 /*
  * INT64
  */
@@ -258,6 +267,26 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+	if (unlikely(a == PG_INT64_MIN))
+	{
+		return true;
+	}
+	else
+	{
+		*result = -a;
+		return false;
+	}
+}
+
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	return a < 0 ? 0 - (uint64) a : (uint64) a;
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +347,25 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+	if (unlikely(a > ((uint16) PG_INT16_MAX) + 1))
+	{
+		return true;
+	}
+	else if (unlikely(a == ((uint16) PG_INT16_MAX) + 1))
+	{
+		*result = PG_INT16_MIN;
+		return false;
+	}
+	else
+	{
+		*result = -((int16) a);
+		return false;
+	}
+}
+
 /*
  * INT32
  */
@@ -373,6 +421,25 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+	if (unlikely(a > ((uint32) PG_INT32_MAX) + 1))
+	{
+		return true;
+	}
+	else if (unlikely(a == ((uint32) PG_INT32_MAX) + 1))
+	{
+		*result = PG_INT32_MIN;
+		return false;
+	}
+	else
+	{
+		*result = -((int32) a);
+		return false;
+	}
+}
+
 /*
  * UINT64
  */
@@ -438,6 +505,25 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+	if (unlikely(a > ((uint64) PG_INT64_MAX) + 1))
+	{
+		return true;
+	}
+	else if (unlikely(a == ((uint64) PG_INT64_MAX) + 1))
+	{
+		*result = PG_INT64_MIN;
+		return false;
+	}
+	else
+	{
+		*result = -((int64) a);
+		return false;
+	}
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..93d4cc323d 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



  [application/x-patch] v18-0005-Prevent-overflow-when-converting-years-to-millenia.patch (3.1K, ../../CAGzNdfFs4hN=_2SZzZivoNtR8H1DiYHc+Zgh_oLv16EgXLuZGg@mail.gmail.com/7-v18-0005-Prevent-overflow-when-converting-years-to-millenia.patch)
  download | inline diff:
From da64d6bb75aed09c296253cffc96c4c5bebc73ad Mon Sep 17 00:00:00 2001
From: Matthew Kim <[email protected]>
Date: Sat, 10 Aug 2024 14:46:43 -0700
Subject: [PATCH 5/5] Prevent overflow when converting years to millenia

Converting a string to timestamp with the Y,YYY template interprets the year with a comma as a thousands separator (for years of 4+ digits). None of the conversion operations handle overflow. This commit checks for overflow when formatting years by millenia (1,000 years).
---
 src/backend/utils/adt/formatting.c     | 10 +++++++++-
 src/test/regress/expected/horology.out |  2 ++
 src/test/regress/sql/horology.sql      |  1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index decf0b6123..e2cb57a9e1 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -3810,7 +3810,15 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
 						ereturn(escontext,,
 								(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
 								 errmsg("invalid input string for \"Y,YYY\"")));
-					years += (millennia * 1000);
+					if (pg_mul_s32_overflow(millennia, 1000, &millennia))
+						ereturn(escontext,,
+								(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+								 errmsg("invalid input string for \"Y,YYY\"")));
+					if (pg_add_s32_overflow(years, millennia, &years))
+						ereturn(escontext,,
+								(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+								 errmsg("invalid input string for \"Y,YYY\"")));
+
 					if (!from_char_set_int(&out->year, years, n, escontext))
 						return;
 					out->yysz = 4;
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 311c688f89..df02d268c0 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3448,6 +3448,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF'
 
 SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
 ERROR:  date/time field value out of range: "2018-11-02 12:34:56.123456789"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR:  invalid input string for "Y,YYY"
 SELECT to_date('1 4 1902', 'Q MM YYYY');  -- Q is ignored
   to_date   
 ------------
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 12a035cf57..db532ee3c0 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -558,6 +558,7 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.1234', 'YYYY-MM-DD HH24:MI:SS.FF' ||
 SELECT i, to_timestamp('2018-11-02 12:34:56.12345', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
 SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
 SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
 
 SELECT to_date('1 4 1902', 'Q MM YYYY');  -- Q is ignored
 SELECT to_date('3 4 21 01', 'W MM CC YY');
-- 
2.39.3 (Apple Git-146)



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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
@ 2024-08-12 10:33   ` jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  1 sibling, 1 reply; 15+ messages in thread

From: jian he @ 2024-08-12 10:33 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Matthew Kim <[email protected]>; Nathan Bossart <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Sat, Aug 10, 2024 at 11:41 PM Joseph Koshakow <[email protected]> wrote:
>
>
>
> On Thu, Aug 8, 2024 at 9:01 PM jian he <[email protected]> wrote:
> >
> > Should the error about integers be out of range?
> >
> > SELECT make_date(-2147483648, 1, 1);
> > "-2147483648" is not an allowed integer.
> >
> > \df make_date
> >                                       List of functions
> >    Schema   |   Name    | Result data type |           Argument data
> > types            | Type
> > ------------+-----------+------------------+------------------------------------------+------
> >  pg_catalog | make_date | date             | year integer, month
> > integer, day integer | func
>
> Are you saying that with the patch applied you're seeing the above
> error? If so, I see a different error.
>
>     test=# SELECT make_date(-2147483648, 1, 1);
>     ERROR:  date field value out of range
>
> Or are you saying that we should change the code in the patch so that
> it returns the above error? If so, I'm not sure I understand the
> reasoning. -2147483648 is an allowed integer, it's the minimum allowed
> value for integers.
>
>     test=# SELECT (-2147483648)::integer;
>         int4
>     -------------
>      -2147483648
>     (1 row)
>

sorry, i mixed up
select (-2147483648)::int;
with
select -2147483648::int;

looks good to me.
maybe make it more explicit:
errmsg("date field (year) value out of range")));
i don't have a huge opinion though.






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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
@ 2024-08-13 21:46     ` Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Nathan Bossart @ 2024-08-13 21:46 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

I've been preparing 0001 for commit.  I've attached what I have so far.

The main changes are the implementations of pg_abs_* and pg_neg_*.  For the
former, I've used abs()/i64abs() for the short/int implementations.  For
the latter, I've tried to use __builtin_sub_overflow() when possible, as
that appears to produce slightly better code.  When
__builtin_sub_overflow() is not available, the values are upcasted before
negation, and we check that result before casting to the return type.  That
approach more closely matches the surrounding functions.  (One exception is
pg_neg_u64_overflow() when we have neither HAVE__BUILTIN_OP_OVERFLOW nor
HAVE_INT128.  In that case, we have to hand-roll everything.)

Thoughts?

-- 
nathan

From c4e67e1f32562037dbc83baca5bcda00e0fcd0d7 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v19 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               | 12 +--
 src/backend/utils/adt/numeric.c            |  4 +-
 src/backend/utils/adt/numutils.c           | 34 ++++----
 src/backend/utils/adt/timestamp.c          | 28 +------
 src/include/common/int.h                   | 92 ++++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +--
 src/test/regress/expected/timestamp.out    | 13 +++
 src/test/regress/expected/timestamptz.out  | 13 +++
 src/test/regress/sql/timestamp.sql         |  4 +
 src/test/regress/sql/timestamptz.sql       |  4 +
 10 files changed, 158 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d0f0923710..3d69e77998 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8117,7 +8117,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11443,7 +11443,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..a3d7d6bf01 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..c76793f72d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+		pg_add_s64_overflow(result, time, &result))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..961e52e9ad 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,11 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - Return the absolute value of a as an unsigned integer of the same
+ * width.
  *---------
  */
 
@@ -97,6 +102,12 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	return abs(a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +165,12 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	return i64abs(a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +275,16 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	if (a < 0)
+		return -a;
+	return a;
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +345,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(a > PG_INT16_MAX))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = (int16) res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +418,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res > PG_INT32_MAX))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = (int32) res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +501,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res > PG_INT64_MAX))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = (int64) res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..93d4cc323d 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



Attachments:

  [text/plain] v19-0001-Remove-dependence-on-integer-wrapping.patch (14.7K, ../../ZrvUOkL19KHEH2sb@nathan/2-v19-0001-Remove-dependence-on-integer-wrapping.patch)
  download | inline diff:
From c4e67e1f32562037dbc83baca5bcda00e0fcd0d7 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v19 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               | 12 +--
 src/backend/utils/adt/numeric.c            |  4 +-
 src/backend/utils/adt/numutils.c           | 34 ++++----
 src/backend/utils/adt/timestamp.c          | 28 +------
 src/include/common/int.h                   | 92 ++++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +--
 src/test/regress/expected/timestamp.out    | 13 +++
 src/test/regress/expected/timestamptz.out  | 13 +++
 src/test/regress/sql/timestamp.sql         |  4 +
 src/test/regress/sql/timestamptz.sql       |  4 +
 10 files changed, 158 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d0f0923710..3d69e77998 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8117,7 +8117,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11443,7 +11443,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..a3d7d6bf01 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..c76793f72d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+		pg_add_s64_overflow(result, time, &result))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..961e52e9ad 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,11 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - Return the absolute value of a as an unsigned integer of the same
+ * width.
  *---------
  */
 
@@ -97,6 +102,12 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	return abs(a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +165,12 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	return i64abs(a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +275,16 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	if (a < 0)
+		return -a;
+	return a;
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +345,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(a > PG_INT16_MAX))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = (int16) res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +418,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res > PG_INT32_MAX))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = (int32) res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +501,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res > PG_INT64_MAX))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = (int64) res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..93d4cc323d 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
@ 2024-08-14 03:07       ` Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Nathan Bossart @ 2024-08-14 03:07 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Tue, Aug 13, 2024 at 04:46:34PM -0500, Nathan Bossart wrote:
> I've been preparing 0001 for commit.  I've attached what I have so far.
> 
> The main changes are the implementations of pg_abs_* and pg_neg_*.  For the
> former, I've used abs()/i64abs() for the short/int implementations.  For
> the latter, I've tried to use __builtin_sub_overflow() when possible, as
> that appears to produce slightly better code.  When
> __builtin_sub_overflow() is not available, the values are upcasted before
> negation, and we check that result before casting to the return type.  That
> approach more closely matches the surrounding functions.  (One exception is
> pg_neg_u64_overflow() when we have neither HAVE__BUILTIN_OP_OVERFLOW nor
> HAVE_INT128.  In that case, we have to hand-roll everything.)

And here's a new version of the patch in which I've attempted to fix the
silly mistakes.

-- 
nathan

From 2b7b38f764f16b49594d0f8cc6192b7ba74ef906 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v20 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               | 12 +--
 src/backend/utils/adt/numeric.c            |  4 +-
 src/backend/utils/adt/numutils.c           | 34 ++++----
 src/backend/utils/adt/timestamp.c          | 28 +------
 src/include/common/int.h                   | 92 ++++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +--
 src/test/regress/expected/timestamp.out    | 13 +++
 src/test/regress/expected/timestamptz.out  | 13 +++
 src/test/regress/sql/timestamp.sql         |  4 +
 src/test/regress/sql/timestamptz.sql       |  4 +
 10 files changed, 158 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d0f0923710..3d69e77998 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8117,7 +8117,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11443,7 +11443,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..a3d7d6bf01 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..c76793f72d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+		pg_add_s64_overflow(result, time, &result))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..7a4dec4635 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,11 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - Return the absolute value of a as an unsigned integer of the same
+ * width.
  *---------
  */
 
@@ -97,6 +102,12 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	return abs(a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +165,12 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	return i64abs(a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +275,16 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	if (a < 0)
+		return -a;
+	return a;
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +345,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(res < PG_INT16_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +418,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res < PG_INT32_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +501,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res < PG_INT64_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..93d4cc323d 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



Attachments:

  [text/plain] v20-0001-Remove-dependence-on-integer-wrapping.patch (14.7K, ../../ZrwfgNyDXy99NUOS@nathan/2-v20-0001-Remove-dependence-on-integer-wrapping.patch)
  download | inline diff:
From 2b7b38f764f16b49594d0f8cc6192b7ba74ef906 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v20 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               | 12 +--
 src/backend/utils/adt/numeric.c            |  4 +-
 src/backend/utils/adt/numutils.c           | 34 ++++----
 src/backend/utils/adt/timestamp.c          | 28 +------
 src/include/common/int.h                   | 92 ++++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +--
 src/test/regress/expected/timestamp.out    | 13 +++
 src/test/regress/expected/timestamptz.out  | 13 +++
 src/test/regress/sql/timestamp.sql         |  4 +
 src/test/regress/sql/timestamptz.sql       |  4 +
 10 files changed, 158 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d0f0923710..3d69e77998 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8117,7 +8117,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11443,7 +11443,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..a3d7d6bf01 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (pg_neg_u16_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (pg_neg_u32_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (pg_neg_u64_overflow(tmp, &result))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..c76793f72d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+		pg_add_s64_overflow(result, time, &result))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..7a4dec4635 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,11 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - Return the absolute value of a as an unsigned integer of the same
+ * width.
  *---------
  */
 
@@ -97,6 +102,12 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	return abs(a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +165,12 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	return i64abs(a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +275,16 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	if (a < 0)
+		return -a;
+	return a;
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +345,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(res < PG_INT16_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +418,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res < PG_INT32_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +501,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res < PG_INT64_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..93d4cc323d 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+		pg_add_s64_overflow(*result, time, result))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
@ 2024-08-14 07:02         ` Heikki Linnakangas <[email protected]>
  2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Heikki Linnakangas @ 2024-08-14 07:02 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; jian he <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On 14/08/2024 06:07, Nathan Bossart wrote:
> On Tue, Aug 13, 2024 at 04:46:34PM -0500, Nathan Bossart wrote:
>> I've been preparing 0001 for commit.  I've attached what I have so far.
>>
>> The main changes are the implementations of pg_abs_* and pg_neg_*.  For the
>> former, I've used abs()/i64abs() for the short/int implementations.  For
>> the latter, I've tried to use __builtin_sub_overflow() when possible, as
>> that appears to produce slightly better code.  When
>> __builtin_sub_overflow() is not available, the values are upcasted before
>> negation, and we check that result before casting to the return type.  That
>> approach more closely matches the surrounding functions.  (One exception is
>> pg_neg_u64_overflow() when we have neither HAVE__BUILTIN_OP_OVERFLOW nor
>> HAVE_INT128.  In that case, we have to hand-roll everything.)
> 
> And here's a new version of the patch in which I've attempted to fix the
> silly mistakes.

LGTM, just a few small comments:

>   * - If a * b overflows, return true, otherwise store the result of a * b
>   * into *result. The content of *result is implementation defined in case of
>   * overflow.
> + * - If -a overflows, return true, otherwise store the result of -a into
> + * *result. The content of *result is implementation defined in case of
> + * overflow.
> + * - Return the absolute value of a as an unsigned integer of the same
> + * width.
>   *---------
>   */

The last "Return the absolute value of a ..." sentence feels a bit 
weird. In all the preceding sentences, 'a' is part of an "If a" sentence 
that defines what 'a' is. In the last one, it's kind of just hanging there.

> +static inline uint16
> +pg_abs_s16(int16 a)
> +{
> +	return abs(a);
> +}
> +

This is correct, but it took me a while to understand why. Maybe some 
comments would be in order.

The function it calls is "int abs(int)". So this first widens the int16 
to int32, and then narrows the result from int32 to uint16.

The man page for abs() says "Trying to take the absolute value of the 
most negative integer is not defined." That's OK in this case, because 
that refers to the most negative int32 value, and the argument here is 
int16. But that's why the pg_abs_s64(int64) function needs the special 
check for the most negative value.


There's also some code in libpq's pqCheckOutBufferSpace() function that 
could use these functions.

-- 
Heikki Linnakangas
Neon (https://neon.tech)







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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
@ 2024-08-14 17:20           ` Nathan Bossart <[email protected]>
  2024-08-14 17:41             ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Nathan Bossart @ 2024-08-14 17:20 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: jian he <[email protected]>; Joseph Koshakow <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Wed, Aug 14, 2024 at 10:02:28AM +0300, Heikki Linnakangas wrote:
> On 14/08/2024 06:07, Nathan Bossart wrote:
>> And here's a new version of the patch in which I've attempted to fix the
>> silly mistakes.
> 
> LGTM, just a few small comments:

Thanks for reviewing.

>>   * - If a * b overflows, return true, otherwise store the result of a * b
>>   * into *result. The content of *result is implementation defined in case of
>>   * overflow.
>> + * - If -a overflows, return true, otherwise store the result of -a into
>> + * *result. The content of *result is implementation defined in case of
>> + * overflow.
>> + * - Return the absolute value of a as an unsigned integer of the same
>> + * width.
>>   *---------
>>   */
> 
> The last "Return the absolute value of a ..." sentence feels a bit weird. In
> all the preceding sentences, 'a' is part of an "If a" sentence that defines
> what 'a' is. In the last one, it's kind of just hanging there.

How about:

	If a is negative, return -a, otherwise return a.  Overflow cannot occur
	because the return value is an unsigned integer with the same width as
	the argument.

>> +static inline uint16
>> +pg_abs_s16(int16 a)
>> +{
>> +	return abs(a);
>> +}
>> +
> 
> This is correct, but it took me a while to understand why. Maybe some
> comments would be in order.
> 
> The function it calls is "int abs(int)". So this first widens the int16 to
> int32, and then narrows the result from int32 to uint16.
> 
> The man page for abs() says "Trying to take the absolute value of the most
> negative integer is not defined." That's OK in this case, because that
> refers to the most negative int32 value, and the argument here is int16. But
> that's why the pg_abs_s64(int64) function needs the special check for the
> most negative value.

Yeah, I've added some casts/comments to make this clear.  I got too excited
about trimming it down and ended up obfuscating these important details.

> There's also some code in libpq's pqCheckOutBufferSpace() function that
> could use these functions.

Duly noted.

-- 
nathan

From 08b32a02629a7628353643960b9956d01a795f18 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v21 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               |  12 ++-
 src/backend/utils/adt/numeric.c            |   4 +-
 src/backend/utils/adt/numutils.c           |  34 ++++---
 src/backend/utils/adt/timestamp.c          |  28 +-----
 src/include/common/int.h                   | 103 +++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c |  11 +--
 src/test/regress/expected/timestamp.out    |  13 +++
 src/test/regress/expected/timestamptz.out  |  13 +++
 src/test/regress/sql/timestamp.sql         |   4 +
 src/test/regress/sql/timestamptz.sql       |   4 +
 10 files changed, 169 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 763a7f4be0..5ee001e382 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8111,7 +8111,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11437,7 +11437,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..63c2beb6a2 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..43800addf4 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+				 pg_add_s64_overflow(result, time, &result)))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..ead74d64d8 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,12 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - If a is negative, return -a, otherwise return a.  Overflow cannot occur
+ * because the return value is an unsigned integer with the same type as the
+ * argument.
  *---------
  */
 
@@ -97,6 +103,17 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	/*
+	 * This first widens the argument from int16 to int32 for use with abs().
+	 * The result is then narrowed from int32 to uint16.  This prevents any
+	 * possibility of overflow.
+	 */
+	return (uint16) abs((int32) a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +171,17 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	/*
+	 * This first widens the argument from int32 to int64 for use with
+	 * i64abs().  The result is then narrowed from int64 to uint32.  This
+	 * prevents any possibility of overflow.
+	 */
+	return (uint32) i64abs((int64) a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +286,16 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	if (a < 0)
+		return -a;
+	return a;
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +356,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(res < PG_INT16_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +429,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res < PG_INT32_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +512,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res < PG_INT64_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..402fae6da6 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (unlikely(pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



Attachments:

  [text/plain] v21-0001-Remove-dependence-on-integer-wrapping.patch (15.3K, ../../ZrznaJTIOPgSjEJ7@nathan/2-v21-0001-Remove-dependence-on-integer-wrapping.patch)
  download | inline diff:
From 08b32a02629a7628353643960b9956d01a795f18 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v21 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               |  12 ++-
 src/backend/utils/adt/numeric.c            |   4 +-
 src/backend/utils/adt/numutils.c           |  34 ++++---
 src/backend/utils/adt/timestamp.c          |  28 +-----
 src/include/common/int.h                   | 103 +++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c |  11 +--
 src/test/regress/expected/timestamp.out    |  13 +++
 src/test/regress/expected/timestamptz.out  |  13 +++
 src/test/regress/sql/timestamp.sql         |   4 +
 src/test/regress/sql/timestamptz.sql       |   4 +
 10 files changed, 169 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 763a7f4be0..5ee001e382 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8111,7 +8111,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11437,7 +11437,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..63c2beb6a2 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..43800addf4 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+				 pg_add_s64_overflow(result, time, &result)))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..ead74d64d8 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,12 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - If a is negative, return -a, otherwise return a.  Overflow cannot occur
+ * because the return value is an unsigned integer with the same type as the
+ * argument.
  *---------
  */
 
@@ -97,6 +103,17 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	/*
+	 * This first widens the argument from int16 to int32 for use with abs().
+	 * The result is then narrowed from int32 to uint16.  This prevents any
+	 * possibility of overflow.
+	 */
+	return (uint16) abs((int32) a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +171,17 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	/*
+	 * This first widens the argument from int32 to int64 for use with
+	 * i64abs().  The result is then narrowed from int64 to uint32.  This
+	 * prevents any possibility of overflow.
+	 */
+	return (uint32) i64abs((int64) a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +286,16 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	if (a < 0)
+		return -a;
+	return a;
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +356,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(res < PG_INT16_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +429,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res < PG_INT32_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +512,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res < PG_INT64_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..402fae6da6 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (unlikely(pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
  2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
@ 2024-08-14 17:41             ` Joseph Koshakow <[email protected]>
  2024-08-14 18:16               ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Joseph Koshakow @ 2024-08-14 17:41 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; jian he <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

Thanks for the improvements Nathan. The current iteration LGTM, just a
single comment on `pg_abs_s64`

> +static inline uint64
> +pg_abs_s64(int64 a)
> +{
> +     if (unlikely(a == PG_INT64_MIN))
> +         return (uint64) PG_INT64_MAX + 1;
> +     if (a < 0)
> +         return -a;
> +     return a;
> +}

Since we know that a does not equal PG_INT64_MIN, could we shorten the
last three lines and do the following?


    static inline uint64
    pg_abs_s64(int64 a)
    {
        if (unlikely(a == PG_INT64_MIN))
          return (uint64) PG_INT64_MAX + 1;
        return i64_abs(a);
    }

Thanks,
Joseph Koshakow


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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
  2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 17:41             ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
@ 2024-08-14 18:16               ` Nathan Bossart <[email protected]>
  2024-08-15 06:56                 ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Nathan Bossart @ 2024-08-14 18:16 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; jian he <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Wed, Aug 14, 2024 at 01:41:40PM -0400, Joseph Koshakow wrote:
> Since we know that a does not equal PG_INT64_MIN, could we shorten the
> last three lines and do the following?
> 
>     static inline uint64
>     pg_abs_s64(int64 a)
>     {
>         if (unlikely(a == PG_INT64_MIN))
>           return (uint64) PG_INT64_MAX + 1;
>         return i64_abs(a);
>     }

Good call.  That actually produces seemingly better code, too.

-- 
nathan

From 75c4c0e0b180f883ea728f45ffe0a81ac2599dcc Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v22 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               |  12 ++-
 src/backend/utils/adt/numeric.c            |   4 +-
 src/backend/utils/adt/numutils.c           |  34 ++++---
 src/backend/utils/adt/timestamp.c          |  28 +-----
 src/include/common/int.h                   | 101 +++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c |  11 +--
 src/test/regress/expected/timestamp.out    |  13 +++
 src/test/regress/expected/timestamptz.out  |  13 +++
 src/test/regress/sql/timestamp.sql         |   4 +
 src/test/regress/sql/timestamptz.sql       |   4 +
 10 files changed, 167 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 763a7f4be0..5ee001e382 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8111,7 +8111,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11437,7 +11437,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..63c2beb6a2 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..43800addf4 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+				 pg_add_s64_overflow(result, time, &result)))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..c76c67e226 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,12 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - If a is negative, return -a, otherwise return a.  Overflow cannot occur
+ * because the return value is an unsigned integer with the same type as the
+ * argument.
  *---------
  */
 
@@ -97,6 +103,17 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	/*
+	 * This first widens the argument from int16 to int32 for use with abs().
+	 * The result is then narrowed from int32 to uint16.  This prevents any
+	 * possibility of overflow.
+	 */
+	return (uint16) abs((int32) a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +171,17 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	/*
+	 * This first widens the argument from int32 to int64 for use with
+	 * i64abs().  The result is then narrowed from int64 to uint32.  This
+	 * prevents any possibility of overflow.
+	 */
+	return (uint32) i64abs((int64) a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +286,14 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	return (uint64) i64abs(a);
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +354,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(res < PG_INT16_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +427,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res < PG_INT32_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +510,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res < PG_INT64_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..402fae6da6 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (unlikely(pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



Attachments:

  [text/plain] v22-0001-Remove-dependence-on-integer-wrapping.patch (15.3K, ../../Zrz0Zv4PVFxieEii@nathan/2-v22-0001-Remove-dependence-on-integer-wrapping.patch)
  download | inline diff:
From 75c4c0e0b180f883ea728f45ffe0a81ac2599dcc Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v22 1/1] Remove dependence on integer wrapping

This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
 src/backend/utils/adt/cash.c               |  12 ++-
 src/backend/utils/adt/numeric.c            |   4 +-
 src/backend/utils/adt/numutils.c           |  34 ++++---
 src/backend/utils/adt/timestamp.c          |  28 +-----
 src/include/common/int.h                   | 101 +++++++++++++++++++++
 src/interfaces/ecpg/pgtypeslib/timestamp.c |  11 +--
 src/test/regress/expected/timestamp.out    |  13 +++
 src/test/regress/expected/timestamptz.out  |  13 +++
 src/test/regress/sql/timestamp.sql         |   4 +
 src/test/regress/sql/timestamptz.sql       |   4 +
 10 files changed, 167 insertions(+), 57 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index ec3c08acfc..c1a743b2a6 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -387,6 +387,7 @@ Datum
 cash_out(PG_FUNCTION_ARGS)
 {
 	Cash		value = PG_GETARG_CASH(0);
+	uint64		uvalue;
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
 
 	if (value < 0)
 	{
-		/* make the amount positive for digit-reconstruction loop */
-		value = -value;
 		/* set up formatting data */
 		signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 		sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
 		sep_by_space = lconvert->p_sep_by_space;
 	}
 
+	/* make the amount positive for digit-reconstruction loop */
+	uvalue = pg_abs_s64(value);
+
 	/* we build the digits+decimal-point+sep string right-to-left in buf[] */
 	bufptr = buf + sizeof(buf) - 1;
 	*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
 			memcpy(bufptr, ssymbol, strlen(ssymbol));
 		}
 
-		*(--bufptr) = ((uint64) value % 10) + '0';
-		value = ((uint64) value) / 10;
+		*(--bufptr) = (uvalue % 10) + '0';
+		uvalue = uvalue / 10;
 		digit_pos--;
-	} while (value || digit_pos >= 0);
+	} while (uvalue || digit_pos >= 0);
 
 	/*----------
 	 * Now, attach currency symbol and sign symbol in the correct order.
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 763a7f4be0..5ee001e382 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8111,7 +8111,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
 	if (val < 0)
 	{
 		var->sign = NUMERIC_NEG;
-		uval = -val;
+		uval = pg_abs_s64(val);
 	}
 	else
 	{
@@ -11437,7 +11437,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
 	 * Now we can proceed with the multiplications.
 	 */
 	neg = (exp < 0);
-	mask = abs(exp);
+	mask = pg_abs_s32(exp);
 
 	init_var(&base_prod);
 	set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..63c2beb6a2 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include "common/int.h"
 #include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 	uint16		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int16		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u16_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int16) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 	uint32		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int32		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u32_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int32) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 	uint64		tmp = 0;
 	bool		neg = false;
 	unsigned char digit;
+	int64		result;
 
 	/*
 	 * The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
 
 	if (neg)
 	{
-		/* check the negative equivalent will fit without overflowing */
-		if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+		if (unlikely(pg_neg_u64_overflow(tmp, &result)))
 			goto out_of_range;
-		return -((int64) tmp);
+		return result;
 	}
 
 	if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..43800addf4 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
 	time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
 			* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
-	result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((result - time) / USECS_PER_DAY != date)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
-						year, month, day,
-						hour, min, sec)));
-
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((result < 0 && date > 0) ||
-		(result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+				 pg_add_s64_overflow(result, time, &result)))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 	date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
 
-	*result = date * USECS_PER_DAY + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != date)
-	{
-		*result = 0;			/* keep compiler quiet */
-		return -1;
-	}
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && date > 0) ||
-		(*result > 0 && date < -1))
+	if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 	{
 		*result = 0;			/* keep compiler quiet */
 		return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..c76c67e226 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,12 @@
  * - If a * b overflows, return true, otherwise store the result of a * b
  * into *result. The content of *result is implementation defined in case of
  * overflow.
+ * - If -a overflows, return true, otherwise store the result of -a into
+ * *result. The content of *result is implementation defined in case of
+ * overflow.
+ * - If a is negative, return -a, otherwise return a.  Overflow cannot occur
+ * because the return value is an unsigned integer with the same type as the
+ * argument.
  *---------
  */
 
@@ -97,6 +103,17 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
 #endif
 }
 
+static inline uint16
+pg_abs_s16(int16 a)
+{
+	/*
+	 * This first widens the argument from int16 to int32 for use with abs().
+	 * The result is then narrowed from int32 to uint16.  This prevents any
+	 * possibility of overflow.
+	 */
+	return (uint16) abs((int32) a);
+}
+
 /*
  * INT32
  */
@@ -154,6 +171,17 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
 #endif
 }
 
+static inline uint32
+pg_abs_s32(int32 a)
+{
+	/*
+	 * This first widens the argument from int32 to int64 for use with
+	 * i64abs().  The result is then narrowed from int64 to uint32.  This
+	 * prevents any possibility of overflow.
+	 */
+	return (uint32) i64abs((int64) a);
+}
+
 /*
  * INT64
  */
@@ -258,6 +286,14 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
 #endif
 }
 
+static inline uint64
+pg_abs_s64(int64 a)
+{
+	if (unlikely(a == PG_INT64_MIN))
+		return (uint64) PG_INT64_MAX + 1;
+	return (uint64) i64abs(a);
+}
+
 /*------------------------------------------------------------------------
  * Overflow routines for unsigned integers
  *------------------------------------------------------------------------
@@ -318,6 +354,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int32		res = -((int32) a);
+
+	if (unlikely(res < PG_INT16_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * INT32
  */
@@ -373,6 +427,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#else
+	int64		res = -((int64) a);
+
+	if (unlikely(res < PG_INT32_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#endif
+}
+
 /*
  * UINT64
  */
@@ -438,6 +510,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
 #endif
 }
 
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+	return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+	uint128		res = -((int128) a);
+
+	if (unlikely(res < PG_INT64_MIN))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	*result = res;
+	return false;
+#else
+	if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+	{
+		*result = 0x5EED;		/* to avoid spurious warnings */
+		return true;
+	}
+	if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+		*result = PG_INT64_MIN;
+	else
+		*result = -((int64) a);
+	return false;
+#endif
+}
+
 /*------------------------------------------------------------------------
  *
  * Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..402fae6da6 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
 #error -ffast-math is known to break this code
 #endif
 
+#include "common/int.h"
 #include "dt.h"
 #include "pgtypes_date.h"
 #include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
 
 	dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
 	time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
-	*result = (dDate * USECS_PER_DAY) + time;
-	/* check for major overflow */
-	if ((*result - time) / USECS_PER_DAY != dDate)
-		return -1;
-	/* check for just-barely overflow (okay except time-of-day wraps) */
-	/* caution: we want to allow 1999-12-31 24:00:00 */
-	if ((*result < 0 && dDate > 0) ||
-		(*result > 0 && dDate < -1))
+	if (unlikely(pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+				 pg_add_s64_overflow(*result, time, result)))
 		return -1;
 	if (tzp != NULL)
 		*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
 
 select age(timestamp '-infinity', timestamp '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+        timestamp         
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+      make_timestamp      
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
 ERROR:  interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+         timestamptz          
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+       make_timestamptz       
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
 select age(timestamp 'infinity', timestamp '-infinity');
 select age(timestamp '-infinity', timestamp 'infinity');
 select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
 SELECT age(timestamptz 'infinity', timestamptz '-infinity');
 SELECT age(timestamptz '-infinity', timestamptz 'infinity');
 SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
-- 
2.39.3 (Apple Git-146)



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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
  2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 17:41             ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-14 18:16               ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
@ 2024-08-15 06:56                 ` jian he <[email protected]>
  2024-08-15 18:45                   ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: jian he @ 2024-08-15 06:56 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Thu, Aug 15, 2024 at 2:16 AM Nathan Bossart <[email protected]> wrote:
>

+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+ uint128 res = -((int128) a);
+
+ if (unlikely(res < PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = res;
+ return false;
+#else
+ if (unlikely(a > (uint64) PG_INT64_MAX + 1))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ if (unlikely(a == (uint64) PG_INT64_MAX + 1))
+ *result = PG_INT64_MIN;
+ else
+ *result = -((int64) a);
+ return false;
+#endif

sorry to bother you.

i am confused with
"
+#elif defined(HAVE_INT128)
+ uint128 res = -((int128) a);
"
I thought "unsigned" means non-negative, therefore uint128 means non-negative.
therefore "int128  res = -((int128) a);" makes sense to me.


also in HAVE_INT128 branch
do we need cast int128 to int64, like

*result = (int64) res;






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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
  2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 17:41             ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-14 18:16               ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-15 06:56                 ` Re: Remove dependence on integer wrapping jian he <[email protected]>
@ 2024-08-15 18:45                   ` Nathan Bossart <[email protected]>
  2024-08-15 21:34                     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Nathan Bossart @ 2024-08-15 18:45 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Thu, Aug 15, 2024 at 02:56:00PM +0800, jian he wrote:
> i am confused with
> "
> +#elif defined(HAVE_INT128)
> + uint128 res = -((int128) a);
> "
> I thought "unsigned" means non-negative, therefore uint128 means non-negative.
> therefore "int128  res = -((int128) a);" makes sense to me.

Ah, that's a typo, thanks for pointing it out.

> also in HAVE_INT128 branch
> do we need cast int128 to int64, like
> 
> *result = (int64) res;

I don't think we need an explicit cast here since *result is known to be an
int64.  But it certainly wouldn't hurt anything...

-- 
nathan






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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
  2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 17:41             ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-14 18:16               ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-15 06:56                 ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-15 18:45                   ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
@ 2024-08-15 21:34                     ` Nathan Bossart <[email protected]>
  2024-08-16 02:49                       ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Nathan Bossart @ 2024-08-15 21:34 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

I've committed 0001.  Now to 0002...

-		if (-element > nelements)
+		if (element == PG_INT32_MIN || -element > nelements)

This seems like a good opportunity to use our new pg_abs_s32() function,
and godbolt.org [0] seems to indicate that it might produce better code,
too (at least to my eye).  I've attached an updated version of the patch
with this change.  Barring additional feedback, I plan to commit this one
shortly.

[0] https://godbolt.org/z/57P4vvGYf

-- 
nathan

From 62677b351cf18dee37dc0d9253bd694fe6fbf26a Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 6 Jul 2024 15:41:09 -0400
Subject: [PATCH v23 1/1] Remove dependence on integer wrapping for jsonb

This commit updates various jsonb operators and functions to no longer
rely on integer wrapping for correctness. Not all compilers support
-fwrapv, so it's best not to rely on it.
---
 src/backend/utils/adt/jsonfuncs.c   |  7 ++++---
 src/test/regress/expected/jsonb.out | 12 ++++++++++++
 src/test/regress/sql/jsonb.sql      |  2 ++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 5ecb9fffae..1f8ea51e6a 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -18,6 +18,7 @@
 
 #include "access/htup_details.h"
 #include "catalog/pg_type.h"
+#include "common/int.h"
 #include "common/jsonapi.h"
 #include "common/string.h"
 #include "fmgr.h"
@@ -946,7 +947,7 @@ jsonb_array_element(PG_FUNCTION_ARGS)
 	{
 		uint32		nelements = JB_ROOT_COUNT(jb);
 
-		if (-element > nelements)
+		if (pg_abs_s32(element) > nelements)
 			PG_RETURN_NULL();
 		else
 			element += nelements;
@@ -5426,7 +5427,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 
 	if (idx < 0)
 	{
-		if (-idx > nelems)
+		if (pg_abs_s32(idx) > nelems)
 		{
 			/*
 			 * If asked to keep elements position consistent, it's not allowed
@@ -5438,7 +5439,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 						 errmsg("path element at position %d is out of range: %d",
 								level + 1, idx)));
 			else
-				idx = INT_MIN;
+				idx = PG_INT32_MIN;
 		}
 		else
 			idx = nelems + idx;
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index e66d760189..a9d93052fc 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -680,6 +680,18 @@ select '"foo"'::jsonb -> 'z';
  
 (1 row)
 
+select '[]'::jsonb -> -2147483648;
+ ?column? 
+----------
+ 
+(1 row)
+
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
+ jsonb_delete_path 
+-------------------
+ {"a": []}
+(1 row)
+
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
  ?column? 
 ----------
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 97bc2242a1..6a18577ead 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -204,6 +204,8 @@ select '[{"b": "c"}, {"b": "cc"}]'::jsonb -> 'z';
 select '{"a": "c", "b": null}'::jsonb -> 'b';
 select '"foo"'::jsonb -> 1;
 select '"foo"'::jsonb -> 'z';
+select '[]'::jsonb -> -2147483648;
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
 
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::int;
-- 
2.39.3 (Apple Git-146)



Attachments:

  [text/plain] v23-0001-Remove-dependence-on-integer-wrapping-for-jsonb.patch (2.9K, ../../Zr50ZpSyrBx0EHfR@nathan/2-v23-0001-Remove-dependence-on-integer-wrapping-for-jsonb.patch)
  download | inline diff:
From 62677b351cf18dee37dc0d9253bd694fe6fbf26a Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 6 Jul 2024 15:41:09 -0400
Subject: [PATCH v23 1/1] Remove dependence on integer wrapping for jsonb

This commit updates various jsonb operators and functions to no longer
rely on integer wrapping for correctness. Not all compilers support
-fwrapv, so it's best not to rely on it.
---
 src/backend/utils/adt/jsonfuncs.c   |  7 ++++---
 src/test/regress/expected/jsonb.out | 12 ++++++++++++
 src/test/regress/sql/jsonb.sql      |  2 ++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 5ecb9fffae..1f8ea51e6a 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -18,6 +18,7 @@
 
 #include "access/htup_details.h"
 #include "catalog/pg_type.h"
+#include "common/int.h"
 #include "common/jsonapi.h"
 #include "common/string.h"
 #include "fmgr.h"
@@ -946,7 +947,7 @@ jsonb_array_element(PG_FUNCTION_ARGS)
 	{
 		uint32		nelements = JB_ROOT_COUNT(jb);
 
-		if (-element > nelements)
+		if (pg_abs_s32(element) > nelements)
 			PG_RETURN_NULL();
 		else
 			element += nelements;
@@ -5426,7 +5427,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 
 	if (idx < 0)
 	{
-		if (-idx > nelems)
+		if (pg_abs_s32(idx) > nelems)
 		{
 			/*
 			 * If asked to keep elements position consistent, it's not allowed
@@ -5438,7 +5439,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 						 errmsg("path element at position %d is out of range: %d",
 								level + 1, idx)));
 			else
-				idx = INT_MIN;
+				idx = PG_INT32_MIN;
 		}
 		else
 			idx = nelems + idx;
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index e66d760189..a9d93052fc 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -680,6 +680,18 @@ select '"foo"'::jsonb -> 'z';
  
 (1 row)
 
+select '[]'::jsonb -> -2147483648;
+ ?column? 
+----------
+ 
+(1 row)
+
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
+ jsonb_delete_path 
+-------------------
+ {"a": []}
+(1 row)
+
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
  ?column? 
 ----------
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 97bc2242a1..6a18577ead 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -204,6 +204,8 @@ select '[{"b": "c"}, {"b": "cc"}]'::jsonb -> 'z';
 select '{"a": "c", "b": null}'::jsonb -> 'b';
 select '"foo"'::jsonb -> 1;
 select '"foo"'::jsonb -> 'z';
+select '[]'::jsonb -> -2147483648;
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
 
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
 select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::int;
-- 
2.39.3 (Apple Git-146)



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

* Re: Remove dependence on integer wrapping
  2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
  2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-14 17:41             ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
  2024-08-14 18:16               ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-15 06:56                 ` Re: Remove dependence on integer wrapping jian he <[email protected]>
  2024-08-15 18:45                   ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
  2024-08-15 21:34                     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
@ 2024-08-16 02:49                       ` Joseph Koshakow <[email protected]>
  0 siblings, 0 replies; 15+ messages in thread

From: Joseph Koshakow @ 2024-08-16 02:49 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>

On Thu, Aug 15, 2024 at 5:34 PM Nathan Bossart <[email protected]>
wrote:

> Now to 0002...
>
> -               if (-element > nelements)
> +               if (element == PG_INT32_MIN || -element > nelements)
>
> This seems like a good opportunity to use our new pg_abs_s32() function,
> and godbolt.org [0] seems to indicate that it might produce better code,
> too (at least to my eye).

This updated version LGTM, I agree it's a good use of pg_abs_s32().

Thanks,
Joseph Koshakow


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

end of thread, other threads:[~2024-08-16 02:49 UTC | newest]

Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-09-25 05:01 [PATCH v8 1/7] Row pattern recognition patch for raw parser. Tatsuo Ishii <[email protected]>
2024-08-09 01:01 Re: Remove dependence on integer wrapping jian he <[email protected]>
2024-08-10 15:41 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
2024-08-10 21:54   ` Re: Remove dependence on integer wrapping Matthew Kim <[email protected]>
2024-08-12 10:33   ` Re: Remove dependence on integer wrapping jian he <[email protected]>
2024-08-13 21:46     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-14 03:07       ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-14 07:02         ` Re: Remove dependence on integer wrapping Heikki Linnakangas <[email protected]>
2024-08-14 17:20           ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-14 17:41             ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
2024-08-14 18:16               ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-15 06:56                 ` Re: Remove dependence on integer wrapping jian he <[email protected]>
2024-08-15 18:45                   ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-15 21:34                     ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-16 02:49                       ` Re: Remove dependence on integer wrapping Joseph Koshakow <[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