support comma in column value for service test csv data

[YannanGao-gs]

What type of PR is this?

Bug Fix

This PR is not backwards compaitble.
Some user tests in prod need to be updated.

What does this PR do / why is it needed ?

Before:

After:

Screen.Recording.2023-06-09.at.6.47.58.PM.mov

Which issue(s) this PR fixes:

Fixes #

Other notes for reviewers:

Does this PR introduce a user-facing change?

[github-actions]

Test Results

     474 files  ±0       474 suites  ±0   1h 2m 49s ⏱️ - 1m 27s
  9 793 tests +2    9 462 ✔️ +2  331 💤 ±0  0 ❌ ±0 
11 655 runs  +2  11 258 ✔️ +2  397 💤 ±0  0 ❌ ±0 

Results for commit bff3d66. ± Comparison against base commit fbfdd87.

♻️ This comment has been updated with latest results.

[ghost]

I think there is issue with the following test case:
'1,Peter's, "I\'m Smith, Jr" ,23,1,1,2'
We need to add logic to handle the case when quote is not the first character

[ghost]

if (!$splitterState.inQuotes && !$splitterState.currentTokenChars->anySatisfy(x| $x != ' '),

[ghost]

Since double-quote is part of data in this case, it should be enclosed in single quote
'!\"#$%&()*+-./:;<=>?@[\]^_`{|}'

[YannanGao-gs]

Hello @vikask-gs and @MauricioUyaguari ,

Could we create a clear rule of database name and column name convention? Currently, we have a very loose standard about how users can define a database name or column name, which is making correctly parsing by comma impossible.

In prod

Since we parse the values from connectionTestData by calling split(',') to separate db names and column values ,

we support

1. database name can be enclosed by double quotes, which enables to have many '\'' and "\"" inside it

There are existing tests (naming.pure) in the codebase

   Table testTable3
    (
      ABCDEFGHIJKLMNOPQRSTUVWXYZ INT PRIMARY KEY,
      "0123456789" INT,
      "!\"#$%&()*+-./:;<=>?@[\]^_`{|}" VARCHAR(20), 
      abcdefghijklmnopqrstuvwxzy INT
    )

3. Each column value in the CSV row is supported to have only one (or many) single quote or double quote

(this example exists in legend-testable-relational-service-embeddedData.pure)

Now

Since we already allow the cases above, there would be many way of parsing the csv data below

1. id = 1 firm_id = 2 firstName = "helloworld, " lastName = again"

2. id = 1 firm_id = 2 firstName = "helloworld lastName = ", again"

No matter how I improve the algorithm, I can't guarantee it could be backward compatible so all existing use cases won't be broken.

Either

A. Could we take a step back to think if it really makes sense to have a delimiter inside the column value?
Currently, the input is the whole string e.g.

There would be tons of corner cases if we allow the cases above

B. Could we make sure that we add the rules below if we have to include a delimiter inside the column value?
Not sure if this will break Prod or not

1.   As long as there is a ' or ", there must be another ' or " to close it. Otherwise use \\' or \\"
 so the current existing test like the below should not be allowed and should be updated 

(this example exists in legend-testable-relational-service-embeddedData.pure)

'1,Peter's, "I\'m Smith, Jr" ,23,1,1,2' should be '1,Peter\'s, "I\'m Smith, Jr" ,23,1,1,2'

2.  if there is a delimiter inside the column value, users have to enclose it with quotes 
 like "test, comma"

[YannanGao-gs]

Spoke to Mauricio offline

1. comma is only wrapped by double quotes
2. if there is an open double quote, it needs to be closed by another double quote

spoke offline with Vikas and Yannan on this PR