Update mssql.py to use QUOTENAME in schema query

[gauiPPP]

QUOTENAME adds [] around the values and handles eg. () in table names etc. With this addition table names will be correct for queries SELECT * FROM [data base 1].[dbo 2].[table name 3] will work

But the current
SELECT * FROM data base 1.dbo 2.table name 3
does not, the spaces lead to error in query

What type of PR is this?

• Refactor
• Feature
• Bug Fix
• New Query Runner (Data Source)
• New Alert Destination
• Other

Description

How is this tested?

• Unit tests (pytest, jest)
• E2E Tests (Cypress)
• Manually
• N/A

SQL query with old and new version, on MS SQL 2022 server

Related Tickets & Documents

I probably have suggested a change to this earlier. The issue has not gone away and QUOTENAME is probable the best solution for this; should work from MS SQL 1996 and up.

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

[cubic-dev-ai]

No issues found across 1 file

[yoshiokatsuneo]

Thank you for your contribution !

Yes, some column names or some table names requires quotes or escapes. And, I see your point.

But, that is not only for Microsoft SQL, but for almost all data source.
I think it is not good idea to add quotes only for Microsoft SQL, when others does not have such a behavior as it is not consistent.

I'm not sure what is the best solution.
Maybe, it is nice if schema browser does not have quotes, and add quotes to SQL only when inserting tables/columns that really requires quotes ?

[gauiPPP]

Thank you for your contribution !

Yes, some column names or some table names requires quotes or escapes. And, I see your point.

But, that is not only for Microsoft SQL, but for almost all data source. I think it is not good idea to add quotes only for Microsoft SQL, when others does not have such a behavior as it is not consistent.

I'm not sure what is the best solution. Maybe, it is nice if schema browser does not have quotes, and add quotes to SQL only when inserting tables/columns that really requires quotes ?

I am not familiar with other databases (that well) nor with the logics of this app more throughly. So this quick fix here and more extensive rework when someone has time and energy?

[yoshiokatsuneo]

I'm sorry I'm not so familiar with Microsoft SQL.
But, I could not find that information that it is common to use bracket for all the tables and columns. So, I'm not sure what is the best solution...

[gauiPPP]

Althouth it might not be common but bracket around the table, schema and database works with all of them still.
I will ask an LLM agent to look at the code and suggest improvements in some later step that would affect more data sources (if needed); in case you are wondering why human input to projects is falling and llm bloat exploding then here you see some explanation; simple partial fix - no no, we demand a thorough solution!

[gauiPPP]

Gemini Code Assist suggest small changes into
redash-master\redash\query_runner_init_.py
and then adding to all the query_runner files 2 rows to define quote identifiers, eg.

phoenix.py
class Phoenix(BaseQueryRunner):
noop_query = "select 1"

def quote_identifier(self, identifier):
    return '"{}"'.format(identifier)

[gauiPPP]

Gemini code assist crashed a few times; there may be some inconsistencies in the files; it made changes to 28 files.
I do not have access to other systems as MS SQL (well fine, postgresql and some others) but extensive testing feels impossible.

[gauiPPP]

"I'm sorry I'm not so familiar with Microsoft SQL.
But, I could not find that information that it is common to use bracket for all the tables and columns. So, I'm not sure what is the best solution..."

Well not using them is currently causing query errors or need to manually add them in editor, which is very frustrating.
So from MS SQL point of view; yes absolutely add them to all, just to be safe, which can be done in the select statement. A separate function after fetching the names could also add the brackets [] to only names that need them. I can suggest such change as well. I may have suggested such a change earlier...

[eradman]

@gauiPPP can you post screen shot of what this error looks like in the Redash UI?

[yoshiokatsuneo]

I think changing all the query from like A to B make the SQL less readable and change the behavior largely.
So I'm sorry but I cannot not support the PR for now...

A:

select * from ec.orders where product = 'aa' order by product

B:

select * from [ec].[orders] where [product] = 'aa' order by [product]

("[procuct]" can be `product` depends on the data source.)

[yoshiokatsuneo]

@eradman

I think PR author get syntax error when one run the SQL for the table/database which contains spaces like below.
And, the PR author does not want to manually add quotes or escape, in my understanding.

[gauiPPP]

I have been working with MS SQL queries soon 10 years and the editor I use has been kind enough to throw in the brackets so I do not get query errors with the spaces.
So I am accustomed to version B.
Keeping that in mind I still have some troube understanding how highlighting table names with [] from SQL query "code parts" can make it less readable. In version A there is no disctinction; I have been trying to implement redash to be used by people with less coding experience and would therefore argue that B is in fact more readable.

A:
select * from ec.orders where product = 'aa' order by product
B:
select * from [ec].[orders] where [product] = 'aa' order by [product]

[gauiPPP]

I suppose I will fork or modify the code when I deploy, which will make updating a bit harder.

I have been wondering why this issue, spaces in table names, has not been taken care of in Redash - or actually in some other SQL querying projects but I am slowly staring to find out:

1. many databases are planned and names given without spaces so this is not a issue
2. suggesting a change only to MS SQL part is a no-go because the code would not be coherent in all different SQL based queries; so instead of making a small change better not make any changes if no 100% solutions is given. I am still a bit confused is the expectation that I should have all different databases running so that I would also test the code against every database / data source
3. I try to think of myself more like "a user with beginner or medium knowledge of code" => so highlighting different parts of queries in different ways for non-coders would be a good thing. Here I seem to face users who feel alright reading SQL queries without even SELECT or WHERE in capital letters so [] is also no go.