From 0dacc4824cf10be6b531e40b6db86c61bebe88ac Mon Sep 17 00:00:00 2001 From: Dariusz Stempniak Date: Tue, 28 May 2024 15:12:03 +0200 Subject: [PATCH] SNOW-1433638 sql trimming only for PUT/GET detection --- .../IntegrationTests/SFDbCommandIT.cs | 50 +++++++++++++++---- .../UnitTests/SFStatementTest.cs | 44 ++++++---------- Snowflake.Data/Core/SFStatement.cs | 44 ++++++++-------- 3 files changed, 79 insertions(+), 59 deletions(-) diff --git a/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs index 3a3cfb82f..8891e2e2a 100755 --- a/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs @@ -129,7 +129,7 @@ public void TestCancelExecuteAsync() } catch { - // assert that cancel is not triggered by timeout, but external cancellation + // assert that cancel is not triggered by timeout, but external cancellation Assert.IsTrue(externalCancel.IsCancellationRequested); } Thread.Sleep(2000); @@ -503,7 +503,7 @@ public void TestRowsAffectedOverflowInt() using (IDbConnection conn = new SnowflakeDbConnection(ConnectionString)) { conn.Open(); - + CreateOrReplaceTable(conn, TableName, new []{"c1 NUMBER"}); using (IDbCommand command = conn.CreateCommand()) @@ -608,7 +608,7 @@ public void TestSimpleLargeResultSet() conn.Close(); } } - + [Test, NonParallelizable] public void TestUseV1ResultParser() { @@ -1021,13 +1021,13 @@ public void testPutArrayBindAsync() private void ArrayBindTest(string connstr, string tableName, int size) { - + CancellationTokenSource externalCancel = new CancellationTokenSource(TimeSpan.FromSeconds(100)); using (DbConnection conn = new SnowflakeDbConnection()) { conn.ConnectionString = connstr; conn.Open(); - + CreateOrReplaceTable(conn, tableName, new [] { "cola INTEGER", @@ -1197,7 +1197,7 @@ public void testExecuteScalarAsyncSelect() { conn.ConnectionString = ConnectionString; conn.Open(); - + CreateOrReplaceTable(conn, TableName, new []{"cola INTEGER"}); using (DbCommand cmd = conn.CreateCommand()) @@ -1624,7 +1624,7 @@ public void TestGetResultsOfUnknownQueryIdWithConfiguredRetry() conn.Close(); } } - + [Test] public void TestSetQueryTagOverridesConnectionString() { @@ -1633,16 +1633,48 @@ public void TestSetQueryTagOverridesConnectionString() string expectedQueryTag = "Test QUERY_TAG 12345"; string connectQueryTag = "Test 123"; conn.ConnectionString = ConnectionString + $";query_tag={connectQueryTag}"; - + conn.Open(); var command = conn.CreateCommand(); ((SnowflakeDbCommand)command).QueryTag = expectedQueryTag; // This query itself will be part of the history and will have the query tag command.CommandText = "SELECT QUERY_TAG FROM table(information_schema.query_history_by_session())"; var queryTag = command.ExecuteScalar(); - + Assert.AreEqual(expectedQueryTag, queryTag); } } + + [Test] + public void TestCommandWithCommentEmbedded() + { + using (var conn = new SnowflakeDbConnection(ConnectionString)) + { + conn.Open(); + var command = conn.CreateCommand(); + + command.CommandText = "\r\nselect '--'\r\n"; + var reader = command.ExecuteReader(); + + Assert.IsTrue(reader.Read()); + Assert.AreEqual("--", reader.GetString(0)); + } + } + + [Test] + public async Task TestCommandWithCommentEmbeddedAsync() + { + using (var conn = new SnowflakeDbConnection(ConnectionString)) + { + conn.Open(); + var command = conn.CreateCommand(); + + command.CommandText = "\r\nselect '--'\r\n"; + var reader = await command.ExecuteReaderAsync().ConfigureAwait(false); + + Assert.IsTrue(await reader.ReadAsync().ConfigureAwait(false)); + Assert.AreEqual("--", reader.GetString(0)); + } + } } } diff --git a/Snowflake.Data.Tests/UnitTests/SFStatementTest.cs b/Snowflake.Data.Tests/UnitTests/SFStatementTest.cs index 24f0f4b0a..3f131e924 100755 --- a/Snowflake.Data.Tests/UnitTests/SFStatementTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SFStatementTest.cs @@ -70,13 +70,10 @@ public void TestServiceName() [Test] public void TestTrimSqlBlockComment() { - Mock.MockRestSessionExpiredInQueryExec restRequester = new Mock.MockRestSessionExpiredInQueryExec(); - SFSession sfSession = new SFSession("account=test;user=test;password=test", null, restRequester); - sfSession.Open(); - SFStatement statement = new SFStatement(sfSession); - SFBaseResultSet resultSet = statement.Execute(0, "/*comment*/select 1/*comment*/", null, false, false); - Assert.AreEqual(true, resultSet.Next()); - Assert.AreEqual("1", resultSet.GetString(0)); + const string SqlSource = "/*comment*/select 1/*comment*/"; + const string SqlExpected = "select 1"; + + Assert.AreEqual(SqlExpected, SFStatement.TrimSql(SqlSource)); } /// @@ -85,13 +82,10 @@ public void TestTrimSqlBlockComment() [Test] public void TestTrimSqlBlockCommentMultiline() { - Mock.MockRestSessionExpiredInQueryExec restRequester = new Mock.MockRestSessionExpiredInQueryExec(); - SFSession sfSession = new SFSession("account=test;user=test;password=test", null, restRequester); - sfSession.Open(); - SFStatement statement = new SFStatement(sfSession); - SFBaseResultSet resultSet = statement.Execute(0, "/*comment\r\ncomment*/select 1/*comment\r\ncomment*/", null, false, false); - Assert.AreEqual(true, resultSet.Next()); - Assert.AreEqual("1", resultSet.GetString(0)); + const string SqlSource = "/*comment\r\ncomment*/select 1/*comment\r\ncomment*/"; + const string SqlExpected = "select 1"; + + Assert.AreEqual(SqlExpected, SFStatement.TrimSql(SqlSource)); } /// @@ -100,13 +94,10 @@ public void TestTrimSqlBlockCommentMultiline() [Test] public void TestTrimSqlLineComment() { - Mock.MockRestSessionExpiredInQueryExec restRequester = new Mock.MockRestSessionExpiredInQueryExec(); - SFSession sfSession = new SFSession("account=test;user=test;password=test", null, restRequester); - sfSession.Open(); - SFStatement statement = new SFStatement(sfSession); - SFBaseResultSet resultSet = statement.Execute(0, "--comment\r\nselect 1\r\n--comment", null, false, false); - Assert.AreEqual(true, resultSet.Next()); - Assert.AreEqual("1", resultSet.GetString(0)); + const string SqlSource = "--comment\r\nselect 1\r\n--comment"; + const string SqlExpected = "select 1\r\n--comment"; + + Assert.AreEqual(SqlExpected, SFStatement.TrimSql(SqlSource)); } /// @@ -115,13 +106,10 @@ public void TestTrimSqlLineComment() [Test] public void TestTrimSqlLineCommentWithClosingNewline() { - Mock.MockRestSessionExpiredInQueryExec restRequester = new Mock.MockRestSessionExpiredInQueryExec(); - SFSession sfSession = new SFSession("account=test;user=test;password=test", null, restRequester); - sfSession.Open(); - SFStatement statement = new SFStatement(sfSession); - SFBaseResultSet resultSet = statement.Execute(0, "--comment\r\nselect 1\r\n--comment\r\n", null, false, false); - Assert.AreEqual(true, resultSet.Next()); - Assert.AreEqual("1", resultSet.GetString(0)); + const string SqlSource = "--comment\r\nselect 1\r\n--comment\r\n"; + const string SqlExpected = "select 1"; + + Assert.AreEqual(SqlExpected, SFStatement.TrimSql(SqlSource)); } [Test] diff --git a/Snowflake.Data/Core/SFStatement.cs b/Snowflake.Data/Core/SFStatement.cs index 787bfa5de..e84690d54 100644 --- a/Snowflake.Data/Core/SFStatement.cs +++ b/Snowflake.Data/Core/SFStatement.cs @@ -110,9 +110,9 @@ class SFStatement private const string SF_QUERY_RESULT_PATH = "/queries/{0}/result"; private const string SF_PARAM_MULTI_STATEMENT_COUNT = "MULTI_STATEMENT_COUNT"; - + private const string SF_PARAM_QUERY_TAG = "QUERY_TAG"; - + private const int SF_QUERY_IN_PROGRESS = 333333; private const int SF_QUERY_IN_PROGRESS_ASYNC = 333334; @@ -124,8 +124,8 @@ class SFStatement private readonly IRestRequester _restRequester; private CancellationTokenSource _timeoutTokenSource; - - // Merged cancellation token source for all cancellation signal. + + // Merged cancellation token source for all cancellation signal. // Cancel callback will be registered under token issued by this source. private CancellationTokenSource _linkedCancellationTokenSource; @@ -151,21 +151,21 @@ internal SFStatement(SFSession session) _restRequester = session.restRequester; _queryTag = session._queryTag; } - - internal SFStatement(SFSession session, string queryTag) + + internal SFStatement(SFSession session, string queryTag) { SfSession = session; _restRequester = session.restRequester; - _queryTag = queryTag ?? session._queryTag; + _queryTag = queryTag ?? session._queryTag; } - + internal string GetBindStage() => _bindStage; private void AssignQueryRequestId() { lock (_requestIdLock) { - + if (_requestId != null) { logger.Info("Another query is running."); @@ -207,8 +207,8 @@ private SFRestRequest BuildQueryRequest(string sql, Dictionary(); } bodyParameters[SF_PARAM_QUERY_TAG] = _queryTag; - } + } QueryRequest postBody = new QueryRequest(); postBody.sqlText = sql; @@ -317,7 +317,7 @@ private void SetTimeout(int timeout) this._timeoutTokenSource = timeout > 0 ? new CancellationTokenSource(timeout * 1000) : new CancellationTokenSource(Timeout.InfiniteTimeSpan); } - + /// /// Register cancel callback. Two factors: either external cancellation token passed down from upper /// layer or timeout reached. Whichever comes first would trigger query cancellation. @@ -363,7 +363,7 @@ internal async Task ExecuteAsync(int timeout, string sql, Dicti } registerQueryCancellationCallback(timeout, cancellationToken); - + int arrayBindingThreshold = 0; if (SfSession.ParameterMap.ContainsKey(SFSessionParameter.CLIENT_STAGE_ARRAY_BINDING_THRESHOLD)) { @@ -457,10 +457,10 @@ internal SFBaseResultSet Execute(int timeout, string sql, Dictionary bindings, bool describeOnly) + private SFBaseResultSet ExecuteSqlWithPutGet(int timeout, string sql, string trimmedSql, Dictionary bindings, bool describeOnly) { try { @@ -484,7 +484,7 @@ private SFBaseResultSet ExecuteSqlWithPutGet(int timeout, string sql, Dictionary logger.Debug("PUT/GET queryId: " + (response.data != null ? response.data.queryId : "Unknown")); SFFileTransferAgent fileTransferAgent = - new SFFileTransferAgent(sql, SfSession, response.data, CancellationToken.None); + new SFFileTransferAgent(trimmedSql, SfSession, response.data, CancellationToken.None); // Start the file transfer fileTransferAgent.execute(); @@ -507,7 +507,7 @@ private SFBaseResultSet ExecuteSqlWithPutGet(int timeout, string sql, Dictionary throw new SnowflakeDbException(ex, SFError.INTERNAL_ERROR); } } - + private SFBaseResultSet ExecuteSqlOtherThanPutGet(int timeout, string sql, Dictionary bindings, bool describeOnly, bool asyncExec) { try @@ -562,7 +562,7 @@ private SFBaseResultSet ExecuteSqlOtherThanPutGet(int timeout, string sql, Dicti throw; } } - + internal async Task GetResultWithIdAsync(string resultId, CancellationToken cancellationToken) { var req = BuildResultRequestWithId(resultId); @@ -938,7 +938,7 @@ internal async Task GetQueryStatusAsync(string queryId, Cancellatio /// /// The original sql query. /// The query without the blanks and comments at the beginning. - private string TrimSql(string originalSql) + internal static string TrimSql(string originalSql) { char[] sqlQueryBuf = originalSql.ToCharArray(); var builder = new StringBuilder(); @@ -1054,7 +1054,7 @@ internal SFBaseResultSet ExecuteTransfer(string sql) false); PutGetStageInfo stageInfo = new PutGetStageInfo(); - + SFFileTransferAgent fileTransferAgent = new SFFileTransferAgent(sql, SfSession, response.data, ref _uploadStream, _destFilename, _stagePath, CancellationToken.None);