Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time data type precision difference between SQL and PostgreSQL #91

Open
psrinivasa2 opened this issue Nov 14, 2024 · 14 comments
Open

Time data type precision difference between SQL and PostgreSQL #91

psrinivasa2 opened this issue Nov 14, 2024 · 14 comments
Labels
bug Something isn't working fixed The bug was fixed in released version

Comments

@psrinivasa2
Copy link

Hi @staticlibs ,
Good Day!

We are facing a error during import that 'Datetime field overflow'. When I further investigate, I could see that time data type precision limit in SQL has 7 and postgreSQL equivalent table has 6.

Any idea on how to fix this issue?

Error during import

image

SQL
image

PostgreSQL
image

@staticlibs
Copy link
Contributor

@psrinivasa2

Datetime precision 6 is caused by the fact that T-SQL datetimes are stored as native Postgres datetimes. And the max precision Postgres supports is 6. Though this should not cause errors, as precision 7 is normally handled as it was specified as 6 in Babelfish. The overflow can be caused by datetime values outside of allowed range. I will recheck this. I also wonder whether you can collect a backtrace for this error?

@psrinivasa2
Copy link
Author

Yeah, I will check the log file with debug installer and let you know Alex

@staticlibs staticlibs added the bug Something isn't working label Nov 14, 2024
@staticlibs
Copy link
Contributor

@psrinivasa2

Please disregard the previous comment, in this case this is actually a 7 precision handling bug in bulk import, in parameters passing. Going to fix this in bulk import.

@psrinivasa2
Copy link
Author

okay thanks

@staticlibs
Copy link
Contributor

@psrinivasa2

The problem appeared to be happening on client side, inside the bcp utility. I've reported it upstream in #3120, see details there. There is no easy fix on server side and currently there are no workarounds on client side with wdb_transfer. Directly with bcp the workaround is to use a custom query on export from MSSQL like this:

bcp "select cast(col1 as time(6)) from tab1" queryout tab1.bcp -n -S 127.0.0.1,1433 -U sa -P pwd

For wdb_transfer to work with Babelfish it is necessary to export time(7) columns from MSSQL as time(6), that requires a major rework in wdb_transfer. It uses bcp (instead of using bulk copy TDS calls directly) exactly to export the table as a whole in a foolproof way and to not deal with columns/types. It is not clear where it is feasible to rework it to support custom column types (to be able to automatically replace time(7) with time(6) on export).

@staticlibs staticlibs added the wdb_transfer Issues related to wdb_transfer label Nov 14, 2024
@staticlibs
Copy link
Contributor

@psrinivasa2

There seems to be a viable workaround on server side, for it to work the column needs to be created in T-SQL as time(7), so its type modifier 7 will be ignored and recorded as -1. This is current behaviour I observe with such columns, but it does not match your second screenshot where time(6) is shown for some reason in PgAdmin.

To apply the workaround run the following SQL in PgAdmin in wilton DB:

CREATE OR REPLACE FUNCTION sys.tsql_type_scale_helper(IN type TEXT, IN typemod INT, IN return_null_for_rest bool) RETURNS sys.TINYINT
AS $$
DECLARE
	scale INT;
BEGIN
	IF type IS NULL THEN 
		RETURN -1;
	END IF;

	IF typemod = -1 THEN
		CASE type
		WHEN 'date' THEN scale = 0;
		WHEN 'datetime' THEN scale = 3;
		WHEN 'smalldatetime' THEN scale = 0;
		WHEN 'datetime2' THEN scale = 7;
		WHEN 'datetimeoffset' THEN scale = 7;
		WHEN 'decimal' THEN scale = 38;
		WHEN 'numeric' THEN scale = 38;
		WHEN 'money' THEN scale = 4;
		WHEN 'smallmoney' THEN scale = 4;
		WHEN 'time' THEN scale = 7;
		WHEN 'tinyint' THEN scale = 0;
		ELSE
			IF return_null_for_rest
				THEN scale = NULL;
			ELSE scale = 0;
			END IF;
		END CASE;
		RETURN scale;
	END IF;

	CASE type 
	WHEN 'decimal' THEN scale = (typemod - 4) & 65535;
	WHEN 'numeric' THEN scale = (typemod - 4) & 65535;
	WHEN 'smalldatetime' THEN scale = 0;
	WHEN 'datetime2' THEN
		CASE typemod 
		WHEN 0 THEN scale = 0;
		WHEN 1 THEN scale = 1;
		WHEN 2 THEN scale = 2;
		WHEN 3 THEN scale = 3;
		WHEN 4 THEN scale = 4;
		WHEN 5 THEN scale = 5;
		WHEN 6 THEN scale = 6;
		-- typemod = 7 is not possible for datetime2 in Babelfish but
		-- adding the case just in case we support it in future
		WHEN 7 THEN scale = 7;
		END CASE;
	WHEN 'datetimeoffset' THEN
		CASE typemod
		WHEN 0 THEN scale = 0;
		WHEN 1 THEN scale = 1;
		WHEN 2 THEN scale = 2;
		WHEN 3 THEN scale = 3;
		WHEN 4 THEN scale = 4;
		WHEN 5 THEN scale = 5;
		WHEN 6 THEN scale = 6;
		-- typemod = 7 is not possible for datetimeoffset in Babelfish
		-- but adding the case just in case we support it in future
		WHEN 7 THEN scale = 7;
		END CASE;
	WHEN 'time' THEN
		CASE typemod
		WHEN 0 THEN scale = 0;
		WHEN 1 THEN scale = 1;
		WHEN 2 THEN scale = 2;
		WHEN 3 THEN scale = 3;
		WHEN 4 THEN scale = 4;
		WHEN 5 THEN scale = 5;
		WHEN 6 THEN scale = 6;
		-- typemod = 7 is not possible for time in Babelfish but
		-- adding the case just in case we support it in future
		WHEN 7 THEN scale = 7;
		END CASE;
	ELSE
		IF return_null_for_rest
			THEN scale = NULL;
		ELSE scale = 0;
		END IF;
	END CASE;
	RETURN scale;
END;
$$ LANGUAGE plpgsql IMMUTABLE STRICT;

@psrinivasa2
Copy link
Author

Thanks @staticlibs , Let me check

@psrinivasa2
Copy link
Author

Hi @staticlibs ,

You can try to recreate it with below query

`

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [AttendanceRegister].[AttendanceRegisterTest](
[TimeSignedIn] time NOT NULL,
[ActualTimeSignedIn] time NOT NULL,
[TimeSignedOut] time NULL,
[ActualTimeSignedOut] time NULL)
GO
`

image

image

@staticlibs
Copy link
Contributor

@psrinivasa2

Running the following on version 12.16.1:

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [AttendanceRegisterTest](
    [TimeSignedIn] time(7) NOT NULL,
    [ActualTimeSignedIn] time(7) NOT NULL,
    [TimeSignedOut] time(7) NULL,
    [ActualTimeSignedOut] time(7) NULL)
GO

I can see the expected -1 type modifier in Postgres:

atttypmod

Confirming this in catalogs:

select a.attname, a.atttypmod 
from pg_catalog.pg_attribute a
join pg_catalog.pg_class c on c.oid = a.attrelid  
where c.relname = 'attendanceregistertest'
attname                        atttypmod
------------------------------ -----------
tableoid                       -1
cmax                           -1
xmax                           -1
cmin                           -1
xmin                           -1
ctid                           -1
timesignedin                   -1
actualtimesignedin             -1
timesignedout                  -1
actualtimesignedout            -1

@psrinivasa2
Copy link
Author

psrinivasa2 commented Nov 18, 2024

Hi @staticlibs ,

Can you please give a try on the below attached whole query?

Datetime prescision issue.txt

@staticlibs
Copy link
Contributor

@psrinivasa2

Thanks for the details! Reproduced the problem, the following with unquoted type name in DDL creates a column with type modifier -1:

create table tab1(
    col1 time(7)
)
select a.attname, a.atttypmod 
from pg_catalog.pg_attribute a
join pg_catalog.pg_class c on c.oid = a.attrelid  
where c.relname = 'tab1'
and a.attname = 'col1'
attname    atttypmod
---------- -----------
col1       -1

But if the typename is quoted in T-SQL style, then the typemodifier is recorded as 6:

create table tab2(
    col2 [time](7)
)
select a.attname, a.atttypmod 
from pg_catalog.pg_attribute a
join pg_catalog.pg_class c on c.oid = a.attrelid  
where c.relname = 'tab2'
and a.attname = 'col2'
attname    atttypmod
---------- -----------
col2       6

I see from where this discrepancy is coming, the time is a native Postgres type, so when it is parsed directly - "invalid" 7 value is discarded. But when the T-SQL logic is applied for the quoted variant - 7 is replaced by max allowed 6. Going to fix this now.

@staticlibs
Copy link
Contributor

The problem is fixed in update 13.17.1.

@staticlibs staticlibs added fixed The bug was fixed in released version and removed wdb_transfer Issues related to wdb_transfer labels Nov 18, 2024
@psrinivasa2
Copy link
Author

psrinivasa2 commented Nov 19, 2024

Thanks @staticlibs, Does the above mentioned function sys.tsql_type_scale_helper is still needed? If yes, does this included in the mentioned release 13.17.1 ?

If it is handled in babelfish, can you share me the upstream babelfish pull request link for us to have discussion with AWS too.

@staticlibs
Copy link
Contributor

@psrinivasa2

Does the above mentioned function sys.tsql_type_scale_helper is still needed? If yes, does this included in the mentioned release 13.17.1 ?

Yes, it is included in 13.17.1 MSI with babelfishpg_tsql extension version 3.3.3, you need to run:

ALTER EXTENSION "babelfishpg_tsql" UPDATE TO '3.3.3';

If it is handled in babelfish, can you share me the upstream babelfish pull request link for us to have discussion with AWS too.

Upstream GH issues - #3120, #3128. There are no PRs for them, the patch for the former is provided there inline. The WiltonDB fix for the latter (engine, extension) is not upstreamable, it is too narrow, more wide fix needs to be developed inside the parser to handle time(7) and [time](7) exactly the same way and other types may have similar problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed The bug was fixed in released version
Projects
None yet
Development

No branches or pull requests

2 participants