server/mssql: avoid encoding varchar values while generating SQL

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4314
GitOrigin-RevId: 852bc941782414c7d190c6195ff367493b927639

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Bug fixes and improvements
 
+- server: avoid encoding 'varchar' values to UTF8 in MSSQL backends
 - server: add support for MSSQL event triggers (#7228)
 - server: update pg_dump to be compatible with postgres 14 (#7676)
 - server: fix parsing remote relationship json definition from 1.x server catalog on migration (fix #7906)

server/graphql-engine.cabal

@@ -1125,6 +1125,7 @@ test-suite tests-hspec
     Test.RunSQLSpec
     Test.InsertCheckPermissionSpec
     Test.InsertEnumColumnSpec
+    Test.SQLServer.InsertVarcharColumnSpec
 
 test-suite tests-gdw-api
   import: common-all, common-exe

server/src-lib/Hasura/Backends/MSSQL/FromIr/Constants.hs

@@ -23,7 +23,7 @@ import Hasura.Backends.MSSQL.Types.Internal as TSQL
 import Hasura.Prelude
 
 trueExpression :: Expression
-trueExpression = ValueExpression (ODBC.BoolValue True)
+trueExpression = ValueExpression $ ODBC.BoolValue True
 
 nullExpression :: Expression
 nullExpression = ValueExpression $ ODBC.TextValue "null"

server/src-lib/Hasura/Backends/MSSQL/Instances/Schema.hs

@@ -10,7 +10,6 @@ module Hasura.Backends.MSSQL.Instances.Schema () where
 import Data.Has
 import Data.HashMap.Strict qualified as Map
 import Data.List.NonEmpty qualified as NE
-import Data.Text.Encoding (encodeUtf8)
 import Data.Text.Extended
 import Database.ODBC.SQLServer qualified as ODBC
 import Hasura.Backends.MSSQL.Schema.IfMatched
@@ -239,10 +238,9 @@ msColumnParser columnType (G.Nullability isNullable) =
     -- incorrect, similarly exposing all the integer types as a GraphQL Int
     ColumnScalar scalarType ->
       possiblyNullable scalarType <$> case scalarType of
-        -- bytestring
-        MSSQL.CharType -> pure $ ODBC.ByteStringValue . encodeUtf8 <$> P.string
-        MSSQL.VarcharType -> pure $ ODBC.ByteStringValue . encodeUtf8 <$> P.string
         -- text
+        MSSQL.CharType -> pure $ ODBC.TextValue <$> P.string
+        MSSQL.VarcharType -> pure $ ODBC.TextValue <$> P.string
         MSSQL.WcharType -> pure $ ODBC.TextValue <$> P.string
         MSSQL.WvarcharType -> pure $ ODBC.TextValue <$> P.string
         MSSQL.WtextType -> pure $ ODBC.TextValue <$> P.string

server/src-lib/Hasura/Backends/MSSQL/SQL/Value.hs

@@ -7,14 +7,13 @@ module Hasura.Backends.MSSQL.SQL.Value (txtEncodedColVal) where
 import Data.Text.Encoding (decodeUtf8)
 import Data.Text.Extended
 import Database.ODBC.SQLServer qualified as ODBC
-import Hasura.Backends.MSSQL.Types.Internal (Value)
 import Hasura.GraphQL.Execute.Subscription.Plan ()
 import Hasura.Prelude
 import Hasura.RQL.Types.Column qualified as RQL
 import Hasura.SQL.Backend
 import Hasura.SQL.Value (TxtEncodedVal (..))
 
-txtEncodedVal :: Value -> TxtEncodedVal
+txtEncodedVal :: ODBC.Value -> TxtEncodedVal
 txtEncodedVal ODBC.NullValue = TENull
 txtEncodedVal (ODBC.ByteStringValue b) = TELit $ decodeUtf8 b
 txtEncodedVal (ODBC.TextValue t) = TELit t

server/src-lib/Hasura/Backends/MSSQL/ToQuery.hs

@@ -88,7 +88,7 @@ fromExpression =
     JsonQueryExpression e -> "JSON_QUERY(" <+> fromExpression e <+> ")"
     JsonValueExpression e path ->
       "JSON_VALUE(" <+> fromExpression e <+> fromPath path <+> ")"
-    ValueExpression value -> QueryPrinter (toSql value)
+    ValueExpression value -> QueryPrinter $ toSql value
     AndExpression xs ->
       case xs of
         [] -> truePrinter

server/src-lib/Hasura/Backends/MSSQL/Types/Internal.hs

@@ -99,7 +99,6 @@ module Hasura.Backends.MSSQL.Types.Internal
 where
 
 import Data.Aeson qualified as J
-import Data.Text.Encoding (encodeUtf8)
 import Database.ODBC.SQLServer qualified as ODBC
 import Hasura.Base.Error
 import Hasura.Incremental (Cacheable)
@@ -595,10 +594,9 @@ mkMSSQLScalarTypeName = \case
 
 parseScalarValue :: ScalarType -> J.Value -> Either QErr Value
 parseScalarValue scalarType jValue = case scalarType of
-  -- bytestring
-  CharType -> ODBC.ByteStringValue . encodeUtf8 <$> parseJValue jValue
-  VarcharType -> ODBC.ByteStringValue . encodeUtf8 <$> parseJValue jValue
   -- text
+  CharType -> ODBC.TextValue <$> parseJValue jValue
+  VarcharType -> ODBC.TextValue <$> parseJValue jValue
   TextType -> ODBC.TextValue <$> parseJValue jValue
   WcharType -> ODBC.TextValue <$> parseJValue jValue
   WvarcharType -> ODBC.TextValue <$> parseJValue jValue

server/tests-hspec/Harness/Quoter/Yaml.hs

@@ -17,11 +17,12 @@ import Data.Aeson (Value)
 import Data.Aeson qualified
 import Data.Aeson qualified as Aeson
 import Data.Aeson.Text qualified as Aeson.Text
-import Data.ByteString.Char8 qualified as BS8
 import Data.Conduit (runConduitRes, (.|))
 import Data.Conduit.List qualified as CL
 import Data.HashMap.Strict qualified as HashMap
-import Data.Text.Encoding.Error qualified as T
+import Data.Text qualified as T
+import Data.Text.Encoding (decodeUtf8With, encodeUtf8)
+import Data.Text.Encoding.Error qualified as TE
 import Data.Text.Lazy qualified as LT
 import Data.Vector qualified as Vector
 import Data.Yaml qualified
@@ -138,7 +139,7 @@ templateYaml inputString = do
       )
     |]
   where
-    inputBytes = BS8.pack inputString
+    inputBytes = encodeUtf8 $ T.pack inputString
 
 -- | Process the events as they come in, potentially expanding any
 -- aliases to objects.
@@ -167,7 +168,7 @@ processor =
 -- | Exceptions that will be thrown mercilessly.
 data YamlTemplateException
   = AnchorsAreDisabled
-  | YamlEncodingProblem T.UnicodeException
+  | YamlEncodingProblem TE.UnicodeException
   deriving stock (Show)
   deriving anyclass (Exception)
 
@@ -187,4 +188,4 @@ newtype Visual = Visual {unVisual :: Value}
   deriving stock (Eq)
 
 instance Show Visual where
-  show = BS8.unpack . Data.Yaml.encode . unVisual
+  show = T.unpack . decodeUtf8With TE.lenientDecode . Data.Yaml.encode . unVisual

server/tests-hspec/Test/SQLServer/InsertVarcharColumnSpec.hs

@@ -0,0 +1,169 @@
+{-# LANGUAGE QuasiQuotes #-}
+
+-- | Test inserting non-ASCII characters in @'varchar' column type
+module Test.SQLServer.InsertVarcharColumnSpec (spec) where
+
+import Harness.Backend.Sqlserver qualified as Sqlserver
+import Harness.GraphqlEngine qualified as GraphqlEngine
+import Harness.Quoter.Graphql (graphql)
+import Harness.Quoter.Sql (sql)
+import Harness.Quoter.Yaml (shouldReturnYaml, yaml)
+import Harness.Test.Context qualified as Context
+import Harness.TestEnvironment (TestEnvironment)
+import Test.Hspec (SpecWith, it)
+import Prelude
+
+-- ** Preamble
+
+spec :: SpecWith TestEnvironment
+spec =
+  Context.run
+    [ Context.Context
+        { name = Context.Backend Context.SQLServer,
+          mkLocalTestEnvironment = Context.noLocalTestEnvironment,
+          setup = mssqlSetup,
+          teardown = mssqlTeardown,
+          customOptions = Nothing
+        }
+    ]
+    tests
+
+-- ** Setup and teardown
+
+mssqlSetup :: (TestEnvironment, ()) -> IO ()
+mssqlSetup (testEnv, ()) = do
+  -- Clear metadata and configure the source
+  GraphqlEngine.setSource testEnv Sqlserver.defaultSourceMetadata
+
+  -- Setup DB schema
+  Sqlserver.run_ setupSQL
+
+  -- Track tables
+  GraphqlEngine.postMetadata_
+    testEnv
+    [yaml|
+type: bulk
+args:
+- type: mssql_track_table
+  args:
+    source: mssql
+    table:
+      schema: hasura
+      name: test
+- type: mssql_track_table
+  args:
+    source: mssql
+    table:
+      schema: hasura
+      name: test_bin
+|]
+
+setupSQL :: String
+setupSQL =
+  [sql|
+CREATE TABLE test (
+  id INT PRIMARY KEY,
+  varchar_column varchar(MAX)
+);
+CREATE TABLE test_bin (
+  id INT PRIMARY KEY,
+  varchar_column varchar(MAX) collate SQL_Latin1_General_CP437_BIN
+);
+|]
+
+mssqlTeardown :: (TestEnvironment, ()) -> IO ()
+mssqlTeardown (testEnv, ()) = do
+  -- Untrack table
+  GraphqlEngine.postMetadata_
+    testEnv
+    [yaml|
+type: bulk
+args:
+- type: mssql_untrack_table
+  args:
+    source: mssql
+    table:
+      schema: hasura
+      name: test
+- type: mssql_untrack_table
+  args:
+    source: mssql
+    table:
+      schema: hasura
+      name: test_bin
+|]
+
+  -- Teardown DB schema
+  Sqlserver.run_ teardownSQL
+
+  -- Clear metadata
+  GraphqlEngine.clearMetadata testEnv
+
+teardownSQL :: String
+teardownSQL =
+  [sql|
+DROP TABLE test;
+DROP TABLE test_bin;
+|]
+
+-- * Tests
+
+tests :: Context.Options -> SpecWith TestEnvironment
+tests opts = do
+  it "Insert into varchar column with non ASCII value" $ \testEnv ->
+    shouldReturnYaml
+      opts
+      ( GraphqlEngine.postGraphql
+          testEnv
+          [graphql|
+mutation {
+  insert_hasura_test(
+    objects: [{id: 1, varchar_column: "££££"}]
+  ) {
+    affected_rows
+    returning{
+      id
+      varchar_column
+    }
+  }
+}
+|]
+      )
+      [yaml|
+data:
+  insert_hasura_test:
+    returning:
+    - id: 1
+      varchar_column: "££££"
+    affected_rows: 1
+
+|]
+
+  it "Insert into collated varchar column with non ASCII value" $ \testEnv ->
+    shouldReturnYaml
+      opts
+      ( GraphqlEngine.postGraphql
+          testEnv
+          [graphql|
+mutation {
+  insert_hasura_test_bin(
+    objects: [{id: 1, varchar_column: "££££"}]
+  ) {
+    affected_rows
+    returning{
+      id
+      varchar_column
+    }
+  }
+}
+|]
+      )
+      [yaml|
+data:
+  insert_hasura_test_bin:
+    returning:
+    - id: 1
+      varchar_column: "££££"
+    affected_rows: 1
+
+|]