Implemented CAST from VARBINARY to DATETIME in Babelfish. #3481

Yvinayak07 · 2025-02-10T08:51:18Z

Description

Previously, Babelfish did not support casting from VARBINARY to DATETIME. We have now implemented this functionality to align with T-SQL's capabilities. This new feature allows users to cast VARBINARY data types to DATETIME.

Task: BABEL-3070
Signed-off-by: yashneet vinayak yashneet@amazon.com

Test Scenarios Covered

**Use case based -**Yes
**Boundary conditions -**Yes
**Arbitrary inputs -**Yes
**Negative test cases -**Yes
Minor version upgrade tests -
Major version upgrade tests -
Performance tests -
Tooling impact -
**Client tests -**Yes

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

Update the antlr version to 4.13.2. This dramatically improves ANTLR parsing performance for certain queries, reducing their execution times by up to 40%. Note that this will require developers to have Java 11+ installed in order to develop locally. Also update the github actions to make it so that previous versions of Babelfish will be compiled using the original antlr-4.9.3 version. This is purely a fix for testing. Task: BABEL-5371 Signed-off-by: Jason Teng <jasonten@amazon.com>

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

)" This reverts commit ff0a4d7.

…3067)" This reverts commit a1e6364.

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

…tgresql#3067)"" This reverts commit 62123f0.

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

coveralls · 2025-02-13T02:44:39Z

Pull Request Test Coverage Report for Build 13323779292

Details

26 of 26 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.01%) to 73.849%

Totals
Change from base Build 13197036746:	0.01%
Covered Lines:	43356
Relevant Lines:	58709

💛 - Coveralls

Anikait143 · 2025-02-11T14:26:00Z

contrib/babelfishpg_common/sql/datetime.sql

@@ -312,6 +312,13 @@ LANGUAGE C VOLATILE STRICT PARALLEL SAFE;
 CREATE CAST (TIMESTAMP AS DATETIME)
 WITH FUNCTION sys.timestamp2datetime(TIMESTAMP) AS ASSIGNMENT;

+CREATE OR REPLACE FUNCTION sys.varbinary2datetime(sys.BBF_VARBINARY)
+RETURNS DATETIME


Let's prepend proper schema everywhere

Prepended sys where ever require.

Here we should return sys.DATETIME instead of DATETIME.

Appended sys schema to the object.

Anikait143 · 2025-02-11T14:39:08Z

contrib/babelfishpg_tsql/sql/sys_function_helpers.sql

Have we thoroughly tested all these functions and error scenarios?

Testing of all scenarios done thoroughly

Anikait143 · 2025-02-11T15:22:24Z

test/JDBC/input/functions/test_conv_string_to_datetime-vu-verify.sql

I believe these tests are copied from 5_X. Have we matched the output?

Yes copied from https://github.com/babelfish-for-postgresql/babelfish_extensions/pull/3448/files.

Anikait143 · 2025-02-13T10:29:13Z

test/JDBC/input/babel_datetime-vu-verify.sql

+
+-- Test Varbinary to Datetime conversion using CAST
+-- Min value
+SELECT CAST(0xFFFF2E4600000000 AS DateTime);


Also add tests to test IMPLICIT casting

Added test for this.

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

rohit01010 · 2025-02-13T17:11:54Z

contrib/babelfishpg_common/sql/datetime.sql

+AS 'babelfishpg_common', 'varbinary_datetime'
+LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
+
+CREATE CAST (sys.BBF_VARBINARY AS DATETIME)


Here as well we should cast to sys.DATETIME instead of DATETIME.

Appended sys schema to the object.

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

sumitj824 · 2025-02-18T09:50:54Z

contrib/babelfishpg_common/src/datetime.c

+/* For 4 byte varbinary datetime range from 1900-01-01 00:00:00.000 to 1900-01-01 23:59:59:999*/
+bool
+is_valid_datetime_for_4byte_varbinary(int64 total_usecs)
+{
+	return (total_usecs >= TSQL_DEFAULT_DATETIME && total_usecs <= MAX_4_BYTE_VARBINARY_DATETIME);
+}


This should be macro instead of function.

If it is not getting used at multiple places then macro is also not needed.

sumitj824 · 2025-02-18T09:59:44Z

contrib/babelfishpg_common/src/datetime.c

+	Timestamp		result;
+	unsigned char		*buffer = (unsigned char *)VARDATA_ANY(arg);
+
+	/* TSQL datetime is 8 bytes */


You mean input should be either 4 or 8 bytes, right? Lets update the comment.

KushaalShroff

For testing, please make sure to add the below tests:

NULLs tests, this is different to 0x00
Boundary testing
Overflows
Coercive tests, eg instead of datetime use date or time which I assume have coercive casts
MAX binary type testing

KushaalShroff · 2025-02-18T12:03:55Z

contrib/babelfishpg_common/src/datetime.c

+	unsigned char		*buffer = (unsigned char *)VARDATA_ANY(arg);
+
+	/* TSQL datetime is 8 bytes */
+	if (size != sizeof(int64) && size != sizeof(int32))


As spoken offline, Cast seems to accept multitude of binary lengths, I think MAX ones as well. Lets add support for all lens.

KushaalShroff · 2025-02-18T12:12:28Z

Are we not supporting binary casts?

yashneet vinayak and others added 16 commits February 10, 2025 08:04

Implemented CAST from VARBINARY to DATETIME in Babelfish

23f98da

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

pulling latest pull

40fc7fc

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

adding schedule file in 14_7

48a9eca

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

removing redundant file in 14_17 schedule

bf62659

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

adding before file in 15_5 schedule

d282c3b

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

changing name of before file according to correct version

f6c766b

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

CheckDatetimeRange() have only one argument in 3_X

d3edd97

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

changing before file name in jdbc_schedule ignore

f58fc54

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

fixing convert function

c5ab93c

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

Revert "[OSS-ONLY] Update antlr to 4.13.2 (babelfish-for-postgresql#3067

a1e6364

)" This reverts commit ff0a4d7.

Reapply "[OSS-ONLY] Update antlr to 4.13.2 (babelfish-for-postgresql#…

62123f0

…3067)" This reverts commit a1e6364.

removing redundant function

dc6a5b2

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

Revert "Reapply "[OSS-ONLY] Update antlr to 4.13.2 (babelfish-for-pos…

223c741

…tgresql#3067)"" This reverts commit 62123f0.

adding sql chnages to upgrade script

4c372fa

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

Correcting before file

1286899

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

Anikait143 reviewed Feb 13, 2025

View reviewed changes

adddressing comments

2df9e5d

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

Yvinayak07 force-pushed the BABEL-3070-3_X branch from 7cfbf40 to 2df9e5d Compare February 13, 2025 13:08

rohit01010 reviewed Feb 13, 2025

View reviewed changes

yashneet vinayak added 2 commits February 14, 2025 05:57

appending sys to datatype in upgrade script

7340c48

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

correcting expected result

fa01a8c

Signed-off-by: yashneet vinayak <yashneet@amazon.com>

sumitj824 reviewed Feb 18, 2025

View reviewed changes

KushaalShroff reviewed Feb 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented CAST from VARBINARY to DATETIME in Babelfish. #3481

Implemented CAST from VARBINARY to DATETIME in Babelfish. #3481

Yvinayak07 commented Feb 10, 2025

coveralls commented Feb 13, 2025 •

edited

Loading

Anikait143 Feb 11, 2025

Yvinayak07 Feb 13, 2025

rohit01010 Feb 13, 2025

Yvinayak07 Feb 14, 2025

Anikait143 Feb 11, 2025

Yvinayak07 Feb 13, 2025

Anikait143 Feb 11, 2025

Yvinayak07 Feb 13, 2025

Anikait143 Feb 13, 2025

Yvinayak07 Feb 13, 2025

rohit01010 Feb 13, 2025

Yvinayak07 Feb 14, 2025

sumitj824 Feb 18, 2025

sumitj824 Feb 18, 2025

KushaalShroff left a comment

KushaalShroff Feb 18, 2025

KushaalShroff commented Feb 18, 2025

Implemented CAST from VARBINARY to DATETIME in Babelfish. #3481

Are you sure you want to change the base?

Implemented CAST from VARBINARY to DATETIME in Babelfish. #3481

Conversation

Yvinayak07 commented Feb 10, 2025

Description

Test Scenarios Covered

Check List

coveralls commented Feb 13, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13323779292

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KushaalShroff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KushaalShroff commented Feb 18, 2025

coveralls commented Feb 13, 2025 •

edited

Loading