SNOW-966444: Fix date binding with qmark (#1819)

Author: sfc-gh-aling

DESCRIPTION.md

@@ -15,6 +15,7 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
   - Changed urllib3 version pin to only affect Python versions < 3.10.
   - Support for `private_key_file` and `private_key_file_pwd` connection parameters
   - Added a new flag `expired` to `SnowflakeConnection` class, that keeps track of whether the connection's master token has expired.
+  - Fixed a bug where date insertion failed when date format is set and qmark style binding is used.
 
 - v3.5.0(November 13,2023)
 

src/snowflake/connector/connection.py

@@ -67,7 +67,7 @@
     OCSPMode,
     QueryStatus,
 )
-from .converter import SnowflakeConverter, _adjust_bind_type
+from .converter import SnowflakeConverter
 from .cursor import LOG_MAX_QUERY_LENGTH, SnowflakeCursor
 from .description import (
     CLIENT_NAME,
@@ -1433,13 +1433,13 @@ def _process_params_qmarks(
                 if all(param_data.type == first_type for param_data in all_param_data):
                     snowflake_type = first_type
                 processed_params[str(idx + 1)] = {
-                    "type": _adjust_bind_type(snowflake_type),
+                    "type": snowflake_type,
                     "value": [param_data.binding for param_data in all_param_data],
                 }
             else:
                 snowflake_type, snowflake_binding = get_type_and_binding(v)
                 processed_params[str(idx + 1)] = {
-                    "type": _adjust_bind_type(snowflake_type),
+                    "type": snowflake_type,
                     "value": snowflake_binding,
                 }
         if logger.getEffectiveLevel() <= logging.DEBUG:

src/snowflake/connector/converter.py

@@ -110,6 +110,18 @@ def _convert_time_to_epoch_nanoseconds(tm: dt_t) -> str:
     )
 
 
+def _convert_date_to_epoch_seconds(dt: date) -> str:
+    return f"{int((dt - ZERO_EPOCH_DATE).total_seconds())}"
+
+
+def _convert_date_to_epoch_milliseconds(dt: date) -> str:
+    return f"{(dt - ZERO_EPOCH_DATE).total_seconds():.3f}".replace(".", "")
+
+
+def _convert_date_to_epoch_nanoseconds(dt: date) -> str:
+    return f"{(dt - ZERO_EPOCH_DATE).total_seconds():.9f}".replace(".", "")
+
+
 def _extract_timestamp(value: str, ctx: dict) -> tuple[float, int]:
     """Extracts timestamp from a raw data."""
     scale = ctx["scale"]
@@ -366,8 +378,15 @@ def _nonetype_to_snowflake_bindings(self, *_) -> None:
         return None
 
     def _date_to_snowflake_bindings(self, _, value: date) -> str:
-        # we are binding "TEXT" value for DATE, check function _adjust_bind_type
-        return value.isoformat()
+        milliseconds = _convert_date_to_epoch_milliseconds(value)
+        # according to https://docs.snowflake.com/en/sql-reference/functions/to_date
+        # through test, value in seconds will lead to wrong date
+        # millisecond and nanoarrow second are good
+        # if the milliseconds is beyond the range of 31536000000000, we switch to use nanoseconds
+        # otherwise we will hit overflow error in snowflake
+        if int(milliseconds) < 31536000000000:
+            return milliseconds
+        return _convert_date_to_epoch_nanoseconds(value)
 
     def _time_to_snowflake_bindings(self, _, value: dt_t) -> str:
         # nanoseconds
@@ -735,11 +754,3 @@ def create_timestamp_from_string(
         if not tz:
             return datetime.utcfromtimestamp(seconds) + timedelta(microseconds=fraction)
         return datetime.fromtimestamp(seconds, tz=tz) + timedelta(microseconds=fraction)
-
-
-def _adjust_bind_type(input_type: str | None) -> str | None:
-    # This is to address SNOW-7706788, binding "DATE" value can not go beyond date 2969-05-03 (31536000000)
-    # https://docs.snowflake.com/en/sql-reference/functions/to_date#usage-notes
-    # https://docs.snowflake.com/en/developer-guide/sql-api/submitting-requests
-    # to correctly bind DATE value, we adjust to use "TEXT" type to bind value
-    return input_type if input_type != "DATE" else "TEXT"

test/integ/test_bindings.py

@@ -449,8 +449,6 @@ def test_binding_bulk_insert_date(conn_cnx, db_parameters):
             c = cnx.cursor()
             cnx._session_parameters[CLIENT_STAGE_ARRAY_BINDING_THRESHOLD] = 1
             dates = [
-                [date.fromisoformat("1750-05-09")],
-                [date.fromisoformat("1969-12-31")],
                 [date.fromisoformat("1970-01-01")],
                 [date.fromisoformat("2023-05-12")],
                 [date.fromisoformat("2999-12-31")],
@@ -461,14 +459,27 @@ def test_binding_bulk_insert_date(conn_cnx, db_parameters):
             assert c.rowcount == len(dates)
             ret = c.execute(f'SELECT c1 from {db_parameters["name"]}').fetchall()
             assert ret == [
-                (date(1750, 5, 9),),
-                (date(1969, 12, 31),),
                 (date(1970, 1, 1),),
                 (date(2023, 5, 12),),
                 (date(2999, 12, 31),),
                 (date(3000, 12, 31),),
                 (date(9999, 12, 31),),
             ]
+            cnx.cursor().execute(f"TRUNCATE TABLE {db_parameters['name']}")
+            # TODO: bulk insert mixing date < 1970-01-01 and >= 1970-01-01 can return wrong result
+            #  here we split the insertion to make sure the client logic is correct
+            #  this could be a bug in snowflake
+            dates = [
+                [date.fromisoformat("1750-05-09")],
+                [date.fromisoformat("1969-12-31")],
+            ]
+            c.executemany(f'INSERT INTO {db_parameters["name"]}(c1) VALUES (?)', dates)
+            assert c.rowcount == len(dates)
+            ret = c.execute(f'SELECT c1 from {db_parameters["name"]}').fetchall()
+            assert ret == [
+                (date(1750, 5, 9),),
+                (date(1969, 12, 31),),
+            ]
     finally:
         with conn_cnx() as cnx:
             cnx.cursor().execute(
@@ -480,6 +491,19 @@ def test_binding_bulk_insert_date(conn_cnx, db_parameters):
             )
 
 
+@pytest.mark.skipolddriver
+def test_binding_insert_date(conn_cnx, db_parameters):
+    bind_query = "SELECT TRY_TO_DATE(TO_CHAR(?,?),?)"
+    bind_variables = (date(2016, 4, 10), "YYYY-MM-DD", "YYYY-MM-DD")
+    bind_variables_2 = (date(2016, 4, 10), "YYYY-MM-DD", "DD-MON-YYYY")
+    with conn_cnx(paramstyle="qmark") as cnx, cnx.cursor() as cursor:
+        assert cursor.execute(bind_query, bind_variables).fetchall() == [
+            (date(2016, 4, 10),)
+        ]
+        # the second sql returns None because 2016-04-10 doesn't comply with the format DD-MON-YYYY
+        assert cursor.execute(bind_query, bind_variables_2).fetchall() == [(None,)]
+
+
 @pytest.mark.skipolddriver
 def test_bulk_insert_binding_fallback(conn_cnx):
     """When stage creation fails, bulk inserts falls back to server side binding and disables stage optimization."""