Fixed Cast from sys.varchar to TIME shows unexpected result due to un…

…defined typmod (babelfish-for-postgresql#2877)

While casting from varchar to TIME, shows unexpected result due to undefined typmod.
It was discovered that some test related to TIME fails randomly on arch64 machine and passed on other 32 bit machines.For query like -- select cast(cast('12:45:37.123' as varchar) as time)

Output :

32bit machines: 12:45:37.1230000
arch64: 12:45:37.0000000

Expected result : 12:45:37.1230000

Reason: Junk value for arch32 is set to a value greater then 7. So it will read real value till 7, Whereas in arch64 junk value is set to 0 due to some compiler optimisation. It won't read any value after '.' setting it to zero.

Changes made to fix the issues --

time_in function was called from varchar2time function with one argument whereas time_in function expects 3 arguments, 3rd one beign the typmod. As this argument was not passed it was picking junk value. This is fixed by calculating tymod in varchar2time and passing correct value of typmod to time_in function.
Also added test cases for this purpose.

Task: BABEL-5179
Signed-off-by: yashneet vinayak <[email protected]>

contrib/babelfishpg_common/Makefile

@@ -60,7 +60,8 @@ GENERATED_UPGRADES = \
     sql/$(EXTENSION)--3.3.0--3.4.0.sql \
     sql/$(EXTENSION)--4.0.0--4.1.0.sql \
     sql/$(EXTENSION)--4.1.0--4.2.0.sql \
-    sql/$(EXTENSION)--4.2.0--4.3.0.sql
+    sql/$(EXTENSION)--4.2.0--4.3.0.sql \
+    sql/$(EXTENSION)--4.3.0--4.4.0.sql
 
 ifdef PREV_EXTVERSION
 DATA = sql/$(EXTENSION)--$(PREV_EXTVERSION).sql
@@ -123,6 +124,8 @@ ifeq (,$(filter $(FLAG_TO_CHECK),$(PG_CPPFLAGS)))
     sql/$(EXTENSION)--4.2.0--4.3.0.sql: sql/upgrades/babelfishpg_upgrades.in
 		cpp -D PREV_VERSION=4.2.0 -D CUR_VERSION=4.3.0 $< | sed 's/^# /-- /g' > $@
 
+    sql/$(EXTENSION)--4.3.0--4.4.0.sql: sql/upgrades/babelfishpg_upgrades.in
+		cpp -D PREV_VERSION=4.3.0 -D CUR_VERSION=4.4.0 $< | sed 's/^# /-- /g' > $@
 else
 # The flag is present build the .in file including the spatial type files
     sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).in
@@ -146,6 +149,9 @@ else
     sql/$(EXTENSION)--4.2.0--4.3.0.sql: sql/upgrades/babelfishpg_upgrades.in
 		cpp -D ENABLE_SPATIAL_TYPES=1 -D PREV_VERSION=4.2.0 -D CUR_VERSION=4.3.0 $< | sed 's/^# /-- /g' > $@
 
+    sql/$(EXTENSION)--4.3.0--4.4.0.sql: sql/upgrades/babelfishpg_upgrades.in
+		cpp -D ENABLE_SPATIAL_TYPES=1 -D PREV_VERSION=4.3.0 -D CUR_VERSION=4.4.0 $< | sed 's/^# /-- /g' > $@
+
 endif
 
 sql/babelfishpg_common--%.sql: sql/upgrades/babelfishpg_common--%.sql

contrib/babelfishpg_common/Version.config

@@ -1,4 +1,4 @@
 BBFPGCMN_MAJOR_VERSION=4
-BBFPGCMN_MINOR_VERSION=3
+BBFPGCMN_MINOR_VERSION=4
 BBFPGCMN_MICRO_VERSION=0
 

contrib/babelfishpg_common/sql/upgrades/babelfish_common_helper--4.3.0--4.4.0.sql

@@ -0,0 +1,71 @@
+------------------------------------------------------------------------------
+---- Include changes related to other datatypes except spatial types here ----
+------------------------------------------------------------------------------
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION ""babelfishpg_common"" UPDATE TO "4.4.0"" to load this file. \quit
+
+SELECT set_config('search_path', 'sys, '||current_setting('search_path'), false);
+
+-- Drops an object if it does not have any dependent objects.
+-- Is a temporary procedure for use by the upgrade script. Will be dropped at the end of the upgrade.
+-- Please have this be one of the first statements executed in this upgrade script. 
+CREATE OR REPLACE PROCEDURE babelfish_drop_deprecated_object(object_type varchar, schema_name varchar, object_name varchar) AS
+$$
+DECLARE
+    error_msg text;
+    query1 text;
+    query2 text;
+BEGIN
+
+    query1 := pg_catalog.format('alter extension babelfishpg_common drop %s %s.%s', object_type, schema_name, object_name);
+    query2 := pg_catalog.format('drop %s %s.%s', object_type, schema_name, object_name);
+
+    execute query1;
+    execute query2;
+EXCEPTION
+    when object_not_in_prerequisite_state then --if 'alter extension' statement fails
+        GET STACKED DIAGNOSTICS error_msg = MESSAGE_TEXT;
+        raise warning '%', error_msg;
+    when dependent_objects_still_exist then --if 'drop view' statement fails
+        GET STACKED DIAGNOSTICS error_msg = MESSAGE_TEXT;
+        raise warning '%', error_msg;
+    when undefined_function then --if 'Deprecated function doen't exsist'
+        GET STACKED DIAGNOSTICS error_msg = MESSAGE_TEXT;
+        raise warning '%', error_msg;
+end
+$$
+LANGUAGE plpgsql;
+
+-- (sys.VARCHAR AS pg_catalog.TIME)
+DROP CAST (sys.VARCHAR AS pg_catalog.TIME);
+
+DO $$    
+DECLARE	
+    exception_message text;	
+BEGIN	
+    ALTER FUNCTION sys.varchar2time(sys.VARCHAR) RENAME TO varchar2time_deprecated_4_4_0;	
+
+EXCEPTION WHEN OTHERS THEN	
+    GET STACKED DIAGNOSTICS	
+    exception_message = MESSAGE_TEXT;	
+    RAISE WARNING '%', exception_message;	
+END;	
+$$;
+
+CREATE OR REPLACE FUNCTION sys.varchar2time(sys.VARCHAR, INT4)
+RETURNS pg_catalog.TIME
+AS 'babelfishpg_common', 'varchar2time'
+LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
+
+CREATE CAST (sys.VARCHAR AS pg_catalog.TIME)
+WITH FUNCTION sys.varchar2time(sys.VARCHAR, INT4) AS IMPLICIT;
+
+CALL sys.babelfish_drop_deprecated_object('function', 'sys', 'varchar2time_deprecated_4_4_0');
+
+-- Drops the temporary procedure used by the upgrade script.
+-- Please have this be one of the last statements executed in this upgrade script.
+DROP PROCEDURE sys.babelfish_drop_deprecated_object(varchar, varchar, varchar);
+
+-- Reset search_path to not affect any subsequent scripts
+SELECT set_config('search_path', trim(leading 'sys, ' from current_setting('search_path')), false);

contrib/babelfishpg_common/sql/upgrades/babelfishpg_common--3.7.0--4.0.0.sql

@@ -0,0 +1,2 @@
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION ""babelfishpg_common"" UPDATE TO '4.0.0'" to load this file. \quit

contrib/babelfishpg_common/sql/upgrades/spatial_types--4.3.0--4.4.0.sql

@@ -0,0 +1,3 @@
+-------------------------------------------------------
+---- Include changes related to spatial types here ----
+-------------------------------------------------------

contrib/babelfishpg_common/sql/varchar.sql

@@ -278,13 +278,13 @@ LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 CREATE CAST (sys.VARCHAR AS pg_catalog.DATE)
 WITH FUNCTION sys.varchar2date(sys.VARCHAR) AS IMPLICIT;
 
-CREATE OR REPLACE FUNCTION sys.varchar2time(sys.VARCHAR)
+CREATE OR REPLACE FUNCTION sys.varchar2time(sys.VARCHAR, INT4)
 RETURNS pg_catalog.TIME
 AS 'babelfishpg_common', 'varchar2time'
 LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
 CREATE CAST (sys.VARCHAR AS pg_catalog.TIME)
-WITH FUNCTION sys.varchar2time(sys.VARCHAR) AS IMPLICIT;
+WITH FUNCTION sys.varchar2time(sys.VARCHAR, INT4) AS IMPLICIT;
 
 CREATE OR REPLACE FUNCTION sys.varchar2money(sys.VARCHAR)
 RETURNS sys.FIXEDDECIMAL

contrib/babelfishpg_common/src/varchar.c

@@ -888,11 +888,15 @@ Datum
 varchar2time(PG_FUNCTION_ARGS)
 {
 	VarChar    *source = PG_GETARG_VARCHAR_PP(0);
+	int32		typmod = -1;
 	char	   *str;
 	TimeADT		time;
+
+	if (PG_NARGS() > 1)
+		typmod = PG_GETARG_INT32(1);
 
 	str = varchar2cstring(source);
-	time = DatumGetTimeADT(DirectFunctionCall1(time_in, CStringGetDatum(str)));
+	time = DatumGetTimeADT(DirectFunctionCall3(time_in, CStringGetDatum(str), InvalidOid, typmod));
 	pfree(str);
 	PG_RETURN_TIMEADT(time);
 }

test/JDBC/expected/cast-varchar-to-time-vu-cleanup.out

@@ -0,0 +1,29 @@
+drop view babel_5179_v1;
+go
+
+drop view babel_5179_v2;
+go
+
+drop view babel_5179_v11;
+go
+
+drop view babel_5179_v22;
+go
+
+drop table babel_5179_t1;
+go
+
+drop table babel_5179_t2;
+go
+
+drop function babel_5179_f1;
+go
+
+drop function babel_5179_f2;
+go
+
+drop procedure babel_5179_p1;
+go
+
+drop procedure babel_5179_p2;
+go

test/JDBC/expected/cast-varchar-to-time-vu-prepare.out

@@ -0,0 +1,60 @@
+create view babel_5179_v1 as select cast(cast('12:45:37.123' as varchar) as time)
+go
+
+create table babel_5179_t1(col1 varchar(50), col2 as cast(col1 as time), constraint timecmp check(col1 >= '12:45:37.1'));
+go
+
+insert into babel_5179_t1 values ('12:45:37.123');
+insert into babel_5179_t1 values ('12:45:37.12');
+insert into babel_5179_t1 values ('12:45:37.1');
+go
+~~ROW COUNT: 1~~
+
+~~ROW COUNT: 1~~
+
+~~ROW COUNT: 1~~
+
+
+create view babel_5179_v11 as select col1, col2 from babel_5179_t1;
+go
+
+create function babel_5179_f1() returns table 
+as return (
+    select cast(cast('12:45:37.123' as varchar) as time)
+)
+go
+
+create procedure babel_5179_p1 as select cast(cast('12:45:37.123' as varchar) as time)
+go
+
+create view babel_5179_v2 as select cast(cast('12:45:37.123' as varchar) as time(2))
+go
+
+create table babel_5179_t2(col1 varchar(50), col2 as cast(col1 as time(2)), constraint timecmp check(col1 >= '12:45:37.1'));
+go
+
+insert into babel_5179_t2 values ('12:45:37.123');
+insert into babel_5179_t2 values ('12:45:37.12');
+insert into babel_5179_t2 values ('12:45:37.1');
+go
+~~ROW COUNT: 1~~
+
+~~ROW COUNT: 1~~
+
+~~ROW COUNT: 1~~
+
+
+create view babel_5179_v22 as select col1, col2 from babel_5179_t2;
+go
+
+CREATE FUNCTION babel_5179_f2() 
+RETURNS @ResultTable TABLE (result TIME(2)) 
+AS 
+BEGIN 
+INSERT INTO @ResultTable SELECT CAST(CAST('12:45:37.123' AS VARCHAR) AS TIME(2));
+RETURN; 
+END;
+go
+
+create procedure babel_5179_p2 as select cast(cast('12:45:37.123' as varchar) as time(2))
+go