Revert support for case insensitive comparison of POCO fields and table column names #560
Conversation
lucyzhang929
requested review from
Charles-Gagnon,
chlafreniere and
MaddyDev
as code owners
|
@lucyzhang929 @Charles-Gagnon is this something that we'd like to get in for the next release? |
|
Yeah, I think we should, if anything just to have as much time as possible with people using it. |
| this.StartFunctionHost(nameof(AddProductParams), lang, false, null, environmentVariables); | ||
|
|
||
| // Change database collation to case sensitive | ||
| this.ExecuteNonQuery($"ALTER DATABASE {this.DatabaseName} SET Single_User WITH ROLLBACK IMMEDIATE; ALTER DATABASE {this.DatabaseName} COLLATE Latin1_General_CS_AS; ALTER DATABASE {this.DatabaseName} SET Multi_User;"); |
There was a problem hiding this comment.
We should still have tests that :
- Verify a case sensitive database can be used without error (at least one for each binding type)
- Using incorrect casing for a column name throws an error
There was a problem hiding this comment.
I will open a separate PR to add the tests in all languages since this PR is quite large already
|
@dzsquared want to make sure you see this PR as a heads up |
|
While working on the tests in Java, I noticed an input binding with mismatched cases is passing when I expected it to fail. Converting this to draft for now while I re-investigate what was the exact json error I was seeing previously. |
|
Discussed offline - for Java Input Bindings if the casing does not match, there is no error thrown but the mismatched field is returned with default values (ex. all Product objects will be returned with {"ProductID": 0} since it doesn't match "ProductId"). |
| - Currently, there is an error for the IEnumerable. We'll fix this by creating an Employee class. | ||
| - Create a new file and call it 'Employee.cs' | ||
| - Paste the below in the file. These are the column names of our SQL table. | ||
| - Paste the below in the file. These are the column names of our SQL table. Note that the casing of the Object field names and the table column names must match. |
There was a problem hiding this comment.
Can you also add this note in the other languages docs appropriately.
| public class ProductColumnTypes | ||
| { | ||
| public int ProductID { get; set; } | ||
| public int ProductId { get; set; } |
There was a problem hiding this comment.
I have these duplicated in test-outofproc folder as well, probably a good idea to move them to a common project and reference them to avoid these redundant issues, but for now can you please update them there as well, thanks!
…le column names (Azure#560) * column names case sensitive * column names case sensitive * update integration test base * remove case sensitive test * update comment * add casing requirement to readme * fix casing in js and ps samples * fix java + js tests * pr comments
Fixes #411
SqlAsyncCollector.cswhere I reverted changes from Check database collation for case sensitivity #191 and fix casing issue when sql table is case insensitive #235