Skip to content

Commit 0f415ae

Browse files
CopilotgilemosGiovanna RibeiroRubenCerna2079vadeveka
authored
Cherry-pick #2887: Fix stored procedure empty cell bug (#3293)
- [x] Cherry-pick commit 69b0b17 (original fix from #2887) - [x] Reverted additional post-review changes (test rename, comment removal) — branch now matches the original cherry-pick exactly <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > Create a cherry-pick PR of #2887 onto the release/1.7 branch. > > Context: > - Repository: Azure/data-api-builder > - PR to cherry-pick: #2887 > - Target branch: release/1.7 > - Goal: apply the same fix from #2887 (“Fix for stored procedure empty cell bug”) onto release/1.7. > > Requirements: > 1. Inspect PR #2887 to identify all commits included (since PR is merged). > 2. Create a new branch off release/1.7, e.g. `cherry-pick/2887-release-1.7`. > 3. Cherry-pick the relevant commit(s) from PR #2887 onto the new branch. > - If the PR contains multiple commits, cherry-pick all that are required for the fix. > - Resolve any conflicts to preserve release/1.7 behavior. > 4. Ensure the solution builds on .NET 8: > - `dotnet build src/Azure.DataApiBuilder.sln` > 5. Run targeted tests if impacted (at minimum, run the existing unit/integration tests relevant to stored procedure execution if present; otherwise run the existing CI test suite entrypoints that are feasible). > 6. Run formatting if needed: > - `dotnet format src/Azure.DataApiBuilder.sln --verify-no-changes` > 7. Open a pull request targeting `release/1.7`. > - PR title: `Cherry-pick #2887: Fix stored procedure empty cell bug` > - PR description should reference original PR and include a brief summary of changes, test results, and conflict resolution notes (if any). > > Notes: > - Do not introduce new features; only backport the fix. > - Keep commit messages clear; use `cherry-pick` convention if repo uses one. > - Ensure no connection strings or secrets are added. > > > The following is the prior conversation context from the user's chat exploration (may be truncated): > > User: Can you cherry-pick below PR to release/1.7 branch and create a PR > @Azure/data-api-builder/pull/2887 > > Assistant: I’ve queued up the cherry-pick PR workflow for `#2887` targeting `release/1.7`, and I’m currently waiting on your approval/confirmation in the prompt so I can proceed with applying the commits and opening the new PR. > > </details> <!-- START COPILOT CODING AGENT SUFFIX --> *This pull request was created from Copilot chat.* > <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Send tasks to Copilot coding agent from [Slack](https://gh.io/cca-slack-docs) and [Teams](https://gh.io/cca-teams-docs) to turn conversations into code. Copilot posts an update in your thread when it's finished. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Giovanna Ribeiro <44936262+gilemos@users.noreply.github.com> Co-authored-by: Giovanna Ribeiro <gribeiro@microsoft.com> Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Co-authored-by: vadeveka <52937801+vadeveka@users.noreply.github.com>
1 parent 14cb7b1 commit 0f415ae

11 files changed

Lines changed: 187 additions & 98 deletions

File tree

‎src/Core/Resolvers/QueryExecutor.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,12 @@ internal int StreamCharData(DbDataReader dbDataReader, long availableSize, Strin
740740
// else we throw exception.
741741
ValidateSize(availableSize, resultFieldSize);
742742

743+
// If the cell is empty, don't append anything to the resultJsonString and return 0.
744+
if (resultFieldSize == 0)
745+
{
746+
return 0;
747+
}
748+
743749
char[] buffer = new char[resultFieldSize];
744750

745751
// read entire field into buffer and reduce available size.
@@ -766,6 +772,13 @@ internal int StreamByteData(DbDataReader dbDataReader, long availableSize, int o
766772
// else we throw exception.
767773
ValidateSize(availableSize, resultFieldSize);
768774

775+
// If the cell is empty, set resultBytes to an empty array and return 0.
776+
if (resultFieldSize == 0)
777+
{
778+
resultBytes = Array.Empty<byte>();
779+
return 0;
780+
}
781+
769782
resultBytes = new byte[resultFieldSize];
770783

771784
dbDataReader.GetBytes(ordinal: ordinal, dataOffset: 0, buffer: resultBytes, bufferOffset: 0, length: resultBytes.Length);

‎src/Service.Tests/DatabaseSchema-DwSql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,8 @@ VALUES (1, 'Awesome book', 1234),
336336
(17, 'CONN%_CONN', 1234),
337337
(18, '[Special Book]', 1234),
338338
(19, 'ME\YOU', 1234),
339-
(20, 'C:\\LIFE', 1234);
339+
(20, 'C:\\LIFE', 1234),
340+
(21, '', 1234);
340341

341342
INSERT INTO book_website_placements(id, book_id, price) VALUES (1, 1, 100), (2, 2, 50), (3, 3, 23), (4, 5, 33);
342343

‎src/Service.Tests/DatabaseSchema-MsSql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,8 @@ VALUES (1, 'Awesome book', 1234),
531531
(17, 'CONN%_CONN', 1234),
532532
(18, '[Special Book]', 1234),
533533
(19, 'ME\YOU', 1234),
534-
(20, 'C:\\LIFE', 1234);
534+
(20, 'C:\\LIFE', 1234),
535+
(21, '', 1234);
535536
SET IDENTITY_INSERT books OFF
536537

537538
SET IDENTITY_INSERT books_mm ON

‎src/Service.Tests/DatabaseSchema-MySql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,8 @@ INSERT INTO books(id, title, publisher_id)
388388
(17, 'CONN%_CONN', 1234),
389389
(18, '[Special Book]', 1234),
390390
(19, 'ME\\YOU', 1234),
391-
(20, 'C:\\\\LIFE', 1234);
391+
(20, 'C:\\\\LIFE', 1234),
392+
(21, '', 1234);
392393
INSERT INTO book_website_placements(book_id, price) VALUES (1, 100), (2, 50), (3, 23), (5, 33);
393394
INSERT INTO website_users(id, username) VALUES (1, 'George'), (2, NULL), (3, ''), (4, 'book_lover_95'), (5, 'null');
394395
INSERT INTO book_author_link(book_id, author_id) VALUES (1, 123), (2, 124), (3, 123), (3, 124), (4, 123), (4, 124), (5, 126);

‎src/Service.Tests/DatabaseSchema-PostgreSql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,8 @@ INSERT INTO books(id, title, publisher_id)
391391
(17, 'CONN%_CONN', 1234),
392392
(18, '[Special Book]', 1234),
393393
(19, 'ME\YOU', 1234),
394-
(20, 'C:\\LIFE', 1234);
394+
(20, 'C:\\LIFE', 1234),
395+
(21, '', 1234);
395396
INSERT INTO book_website_placements(book_id, price) VALUES (1, 100), (2, 50), (3, 23), (5, 33);
396397
INSERT INTO website_users(id, username) VALUES (1, 'George'), (2, NULL), (3, ''), (4, 'book_lover_95'), (5, 'null');
397398
INSERT INTO book_author_link(book_id, author_id) VALUES (1, 123), (2, 124), (3, 123), (3, 124), (4, 123), (4, 124), (5, 126);;

‎src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ public async Task TestStoredProcedureMutationForDeletion(string dbQueryToVerifyD
257257

258258
string currentDbResponse = await GetDatabaseResultAsync(dbQueryToVerifyDeletion);
259259
JsonDocument currentResult = JsonDocument.Parse(currentDbResponse);
260-
Assert.AreEqual(currentResult.RootElement.GetProperty("maxId").GetInt64(), 20);
260+
Assert.AreEqual(currentResult.RootElement.GetProperty("maxId").GetInt64(), 21);
261261
JsonElement graphQLResponse = await ExecuteGraphQLRequestAsync(graphQLMutation, graphQLMutationName, isAuthenticated: true);
262262

263263
// Stored Procedure didn't return anything
@@ -266,7 +266,7 @@ public async Task TestStoredProcedureMutationForDeletion(string dbQueryToVerifyD
266266
// check to verify new element is inserted
267267
string updatedDbResponse = await GetDatabaseResultAsync(dbQueryToVerifyDeletion);
268268
JsonDocument updatedResult = JsonDocument.Parse(updatedDbResponse);
269-
Assert.AreEqual(updatedResult.RootElement.GetProperty("maxId").GetInt64(), 19);
269+
Assert.AreEqual(updatedResult.RootElement.GetProperty("maxId").GetInt64(), 20);
270270
}
271271

272272
public async Task InsertMutationOnTableWithTriggerWithNonAutoGenPK(string dbQuery)

‎src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs‎

Lines changed: 101 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -84,95 +84,101 @@ public async Task RequestMaxUsingNegativeOne()
8484
}
8585
}";
8686

87-
// this resultset represents all books in the db.
88-
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
8987
string expected = @"{
90-
""items"": [
88+
""items"": [
9189
{
92-
""id"": 1,
93-
""title"": ""Awesome book""
90+
""id"": 1,
91+
""title"": ""Awesome book""
9492
},
9593
{
96-
""id"": 2,
97-
""title"": ""Also Awesome book""
94+
""id"": 2,
95+
""title"": ""Also Awesome book""
9896
},
9997
{
100-
""id"": 3,
101-
""title"": ""Great wall of china explained""
98+
""id"": 3,
99+
""title"": ""Great wall of china explained""
102100
},
103101
{
104-
""id"": 4,
105-
""title"": ""US history in a nutshell""
102+
""id"": 4,
103+
""title"": ""US history in a nutshell""
106104
},
107105
{
108-
""id"": 5,
109-
""title"": ""Chernobyl Diaries""
106+
""id"": 5,
107+
""title"": ""Chernobyl Diaries""
110108
},
111109
{
112-
""id"": 6,
113-
""title"": ""The Palace Door""
110+
""id"": 6,
111+
""title"": ""The Palace Door""
114112
},
115113
{
116-
""id"": 7,
117-
""title"": ""The Groovy Bar""
114+
""id"": 7,
115+
""title"": ""The Groovy Bar""
118116
},
119117
{
120-
""id"": 8,
121-
""title"": ""Time to Eat""
118+
""id"": 8,
119+
""title"": ""Time to Eat""
122120
},
123121
{
124-
""id"": 9,
125-
""title"": ""Policy-Test-01""
122+
""id"": 9,
123+
""title"": ""Policy-Test-01""
126124
},
127125
{
128-
""id"": 10,
129-
""title"": ""Policy-Test-02""
126+
""id"": 10,
127+
""title"": ""Policy-Test-02""
130128
},
131129
{
132-
""id"": 11,
133-
""title"": ""Policy-Test-04""
130+
""id"": 11,
131+
""title"": ""Policy-Test-04""
134132
},
135133
{
136-
""id"": 12,
137-
""title"": ""Time to Eat 2""
134+
""id"": 12,
135+
""title"": ""Time to Eat 2""
136+
},
137+
{
138+
""id"": 13,
139+
""title"": ""Before Sunrise""
138140
},
139141
{
140-
""id"": 13,
141-
""title"": ""Before Sunrise""
142+
""id"": 14,
143+
""title"": ""Before Sunset""
142144
},
143145
{
144-
""id"": 14,
145-
""title"": ""Before Sunset""
146+
""id"": 15,
147+
""title"": ""SQL_CONN""
146148
},
147149
{
148-
""id"": 15,
149-
""title"": ""SQL_CONN""
150+
""id"": 16,
151+
""title"": ""SOME%CONN""
150152
},
151153
{
152-
""id"": 16,
153-
""title"": ""SOME%CONN""
154+
""id"": 17,
155+
""title"": ""CONN%_CONN""
154156
},
155157
{
156-
""id"": 17,
157-
""title"": ""CONN%_CONN""
158+
""id"": 18,
159+
""title"": ""[Special Book]""
158160
},
159161
{
160-
""id"": 18,
161-
""title"": ""[Special Book]""
162+
""id"": 19,
163+
""title"": ""ME\\YOU""
162164
},
163165
{
164-
""id"": 19,
165-
""title"": ""ME\\YOU""
166+
""id"": 20,
167+
""title"": ""C:\\\\LIFE""
166168
},
167169
{
168-
""id"": 20,
169-
""title"": ""C:\\\\LIFE""
170+
""id"": 21,
171+
""title"": """"
170172
}
171-
],
172-
""endCursor"": null,
173-
""hasNextPage"": false
173+
],
174+
""endCursor"": null,
175+
""hasNextPage"": false
174176
}";
175177

178+
// Note: The max page size is 21 for MsSql and 20 for all other data sources, so when using -1
179+
// this resultset represents all books in the db.
180+
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
181+
176182
SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString());
177183
}
178184

@@ -196,91 +202,96 @@ public async Task RequestNoParamFullConnection()
196202
}";
197203

198204
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
205+
199206
string expected = @"{
200-
""items"": [
207+
""items"": [
201208
{
202-
""id"": 1,
203-
""title"": ""Awesome book""
209+
""id"": 1,
210+
""title"": ""Awesome book""
204211
},
205212
{
206-
""id"": 2,
207-
""title"": ""Also Awesome book""
213+
""id"": 2,
214+
""title"": ""Also Awesome book""
208215
},
209216
{
210-
""id"": 3,
211-
""title"": ""Great wall of china explained""
217+
""id"": 3,
218+
""title"": ""Great wall of china explained""
212219
},
213220
{
214-
""id"": 4,
215-
""title"": ""US history in a nutshell""
221+
""id"": 4,
222+
""title"": ""US history in a nutshell""
216223
},
217224
{
218-
""id"": 5,
219-
""title"": ""Chernobyl Diaries""
225+
""id"": 5,
226+
""title"": ""Chernobyl Diaries""
220227
},
221228
{
222-
""id"": 6,
223-
""title"": ""The Palace Door""
229+
""id"": 6,
230+
""title"": ""The Palace Door""
224231
},
225232
{
226-
""id"": 7,
227-
""title"": ""The Groovy Bar""
233+
""id"": 7,
234+
""title"": ""The Groovy Bar""
228235
},
229236
{
230-
""id"": 8,
231-
""title"": ""Time to Eat""
237+
""id"": 8,
238+
""title"": ""Time to Eat""
232239
},
233240
{
234-
""id"": 9,
235-
""title"": ""Policy-Test-01""
241+
""id"": 9,
242+
""title"": ""Policy-Test-01""
236243
},
237244
{
238-
""id"": 10,
239-
""title"": ""Policy-Test-02""
245+
""id"": 10,
246+
""title"": ""Policy-Test-02""
240247
},
241248
{
242-
""id"": 11,
243-
""title"": ""Policy-Test-04""
249+
""id"": 11,
250+
""title"": ""Policy-Test-04""
244251
},
245252
{
246-
""id"": 12,
247-
""title"": ""Time to Eat 2""
253+
""id"": 12,
254+
""title"": ""Time to Eat 2""
248255
},
249256
{
250-
""id"": 13,
251-
""title"": ""Before Sunrise""
257+
""id"": 13,
258+
""title"": ""Before Sunrise""
252259
},
253260
{
254-
""id"": 14,
255-
""title"": ""Before Sunset""
261+
""id"": 14,
262+
""title"": ""Before Sunset""
256263
},
257264
{
258-
""id"": 15,
259-
""title"": ""SQL_CONN""
265+
""id"": 15,
266+
""title"": ""SQL_CONN""
260267
},
261268
{
262-
""id"": 16,
263-
""title"": ""SOME%CONN""
269+
""id"": 16,
270+
""title"": ""SOME%CONN""
264271
},
265272
{
266-
""id"": 17,
267-
""title"": ""CONN%_CONN""
273+
""id"": 17,
274+
""title"": ""CONN%_CONN""
268275
},
269276
{
270-
""id"": 18,
271-
""title"": ""[Special Book]""
277+
""id"": 18,
278+
""title"": ""[Special Book]""
272279
},
273280
{
274-
""id"": 19,
275-
""title"": ""ME\\YOU""
281+
""id"": 19,
282+
""title"": ""ME\\YOU""
276283
},
277284
{
278-
""id"": 20,
279-
""title"": ""C:\\\\LIFE""
285+
""id"": 20,
286+
""title"": ""C:\\\\LIFE""
287+
},
288+
{
289+
""id"": 21,
290+
""title"": """"
280291
}
281-
],
282-
""endCursor"": null,
283-
""hasNextPage"": false
292+
],
293+
""endCursor"": null,
294+
""hasNextPage"": false
284295
}";
285296

286297
SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString());

‎src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,22 @@ SELECT title FROM books
463463
await QueryWithSingleColumnPrimaryKey(msSqlQuery);
464464
}
465465

466+
[TestMethod]
467+
public virtual async Task QueryWithEmptyStringResult()
468+
{
469+
string graphQLQueryName = "book_by_pk";
470+
string graphQLQuery = @"{
471+
book_by_pk(id: 21) {
472+
title
473+
}
474+
}";
475+
476+
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
477+
478+
string title = actual.GetProperty("title").GetString();
479+
Assert.AreEqual("", title);
480+
}
481+
466482
[TestMethod]
467483
public async Task QueryWithSingleColumnPrimaryKeyAndMappings()
468484
{

0 commit comments

Comments
 (0)