Fixing bug with handling output columns - MySql (#1647)

ayush3797 · web-flow · commit ecc0778bd2ba · 2023-08-24T07:14:31.000Z
## Why make this change? Fix #1648 Currently Output columns are incorrectly handled while generating SELECT query for MySql after update via UPSERT operation. Relevant code snippet: <img width="635" alt="image" src="https://github.com/Azure/data-api-builder/assets/34566234/b835c486-0028-4e84-941c-a7eae9cc6916"> Link to file: https://github.com/Azure/data-api-builder/blob/5e3874e6a77179523d270a0358efdcbbea9762fa/src/Core/Resolvers/MySqlQueryBuilder.cs#L293 **Where is the bug?** As we can observe that in the attached code snippet, we are processing the output columns one by one. The IF block for line 293 is hit when we find a column which is present in the `structure.InsertColumns`. We add such a column to the final `selections` and increment the index: `selections.Add($"{structure.Values[index]} AS {quotedColName}");` `index++;` **What have we assumed here?** We have assumed that the order of columns in `structure.InsertColumns` is same as `structure.OutputColumns`. Well that is certainly not true as output columns can have columns other that insertColumns and even if the columns are same, the order can differ. **So, we cannot just reference `structure.Values[index]` on line 293 and increment the index on line 294.**. We need to be sure that this output column, which is also an insert column, corresponds to this parameter name at `structure.Values[index]` (structure.Values essentially stores parameter names for each of the insert columns). ## What is this change? - Instead of incrementing index, we need a dictionary `insertColumnsToParamName` which certainly tells us as to what paramName does each of the insertColumns map to. **This also prevents an O(n) lookup** that we were doing to check if the output column is present in the list of `structure.InsertColumns`. ## How was this tested? Will be adding tests in: #1596, if required at all. The existing tests are still passing which confirms the change is good.
diff --git a/src/Core/Resolvers/MySqlQueryBuilder.cs b/src/Core/Resolvers/MySqlQueryBuilder.cs
@@ -280,26 +280,26 @@ private string MakeInsertSelections(SqlInsertStructure structure)
         private string MakeUpsertSelections(SqlUpsertQueryStructure structure)
         {
             List<string> selections = new();
+            Dictionary<string, string> insertColumnsToParamName = structure.InsertColumns.Zip(structure.Values, (colName, paramName)
+                => new { Key = colName, Value = paramName }).ToDictionary(kv => kv.Key, kv => kv.Value);
 
             List<LabelledColumn> fields = structure.OutputColumns;
 
-            int index = 0;
             foreach (LabelledColumn column in fields)
             {
                 string quotedColName = QuoteIdentifier(column.Label);
-
-                if (structure.InsertColumns.Contains(column.ColumnName))
+                ColumnDefinition columnDefinition = structure.GetColumnDefinition(column.ColumnName);
+                if (insertColumnsToParamName.TryGetValue(column.ColumnName, out string? paramName))
                 {
-                    selections.Add($"{structure.Values[index]} AS {quotedColName}");
-                    index++;
+                    selections.Add($"{paramName} AS {quotedColName}");
                 }
-                else if (structure.GetColumnDefinition(column.ColumnName).IsAutoGenerated)
+                else if (columnDefinition.IsAutoGenerated)
                 {
                     selections.Add($"LAST_INSERT_ID() AS {quotedColName}");
                 }
-                else if (structure.GetColumnDefinition(column.ColumnName).HasDefault)
+                else if (columnDefinition.HasDefault)
                 {
-                    selections.Add($"{GetMySQLDefaultValue(structure.GetColumnDefinition(column.ColumnName))} AS {quotedColName}");
+                    selections.Add($"{GetMySQLDefaultValue(columnDefinition)} AS {quotedColName}");
                 }
                 else
                 {

Original file line number	Diff line number	Diff line change
`@@ -280,26 +280,26 @@ private string MakeInsertSelections(SqlInsertStructure structure)`
`280`	`280`	`private string MakeUpsertSelections(SqlUpsertQueryStructure structure)`
`281`	`281`	`{`
`282`	`282`	`List<string> selections = new();`
	`283`	`+ Dictionary<string, string> insertColumnsToParamName = structure.InsertColumns.Zip(structure.Values, (colName, paramName)`
	`284`	`+ => new { Key = colName, Value = paramName }).ToDictionary(kv => kv.Key, kv => kv.Value);`
`283`	`285`
`284`	`286`	`List<LabelledColumn> fields = structure.OutputColumns;`
`285`	`287`
`286`		`- int index = 0;`
`287`	`288`	`foreach (LabelledColumn column in fields)`
`288`	`289`	`{`
`289`	`290`	`string quotedColName = QuoteIdentifier(column.Label);`
`290`		`-`
`291`		`- if (structure.InsertColumns.Contains(column.ColumnName))`
	`291`	`+ ColumnDefinition columnDefinition = structure.GetColumnDefinition(column.ColumnName);`
	`292`	`+ if (insertColumnsToParamName.TryGetValue(column.ColumnName, out string? paramName))`
`292`	`293`	`{`
`293`		`- selections.Add($"{structure.Values[index]} AS {quotedColName}");`
`294`		`- index++;`
	`294`	`+ selections.Add($"{paramName} AS {quotedColName}");`
`295`	`295`	`}`
`296`		`- else if (structure.GetColumnDefinition(column.ColumnName).IsAutoGenerated)`
	`296`	`+ else if (columnDefinition.IsAutoGenerated)`
`297`	`297`	`{`
`298`	`298`	`selections.Add($"LAST_INSERT_ID() AS {quotedColName}");`
`299`	`299`	`}`
`300`		`- else if (structure.GetColumnDefinition(column.ColumnName).HasDefault)`
	`300`	`+ else if (columnDefinition.HasDefault)`
`301`	`301`	`{`
`302`		`- selections.Add($"{GetMySQLDefaultValue(structure.GetColumnDefinition(column.ColumnName))} AS {quotedColName}");`
	`302`	`+ selections.Add($"{GetMySQLDefaultValue(columnDefinition)} AS {quotedColName}");`
`303`	`303`	`}`
`304`	`304`	`else`
`305`	`305`	`{`