Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug 2177 2168 #2183

Closed
wants to merge 3 commits into from
Closed

fix bug 2177 2168 #2183

wants to merge 3 commits into from

Conversation

taijiyouxia
Copy link

@taijiyouxia taijiyouxia commented Jun 8, 2023

fix #2168
fix #2177
explain plan for create table test (id number);
select CARDINALITY from (select CARDINALITY from PLAN_TABLE t where id = 0 order by t.timestamp desc) where rownum = 1

image

注意上面的返回是有结果的,一行数据,但是这一行数据值为null。
所以rows[0] 为None。

此时会走到
result["rows"] = rows[0]

会报:
#2168
[2023-05-23 16:44:22,753][Thread-2:139803802728192][task_id:default][oracle.py:1010][WARNING]- Oracle 语句执行报错,第1个SQL:create table test111(id int);,错误信息Traceback (most recent call last):
File "/backup/Archery-1.9.1/sql/engines/oracle.py", line 836, in execute_check
if result_set["rows"] > 1000:
TypeError: '>' not supported between instances of 'NoneType' and 'int'

fix bug 2177:
11.2.0.4版本中,不支持ddl中绑定变量,故修改为
cursor.execute(f' ALTER SESSION SET CURRENT_SCHEMA = "{db_name}" ')

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Patch coverage: 33.33% and project coverage change: +0.01 🎉

Comparison is base (cedc1f3) 75.13% compared to head (6e84f59) 75.14%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2183      +/-   ##
==========================================
+ Coverage   75.13%   75.14%   +0.01%     
==========================================
  Files         105      105              
  Lines       15169    15169              
==========================================
+ Hits        11397    11399       +2     
+ Misses       3772     3770       -2     
Impacted Files Coverage Δ
sql/engines/oracle.py 50.68% <33.33%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -609,7 +606,7 @@ def explain_check(self, db_name=None, sql="", close_conn=False):
)
rows = cursor.fetchone()
conn.rollback()
if not rows:
if rows[0] is None or not rows:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if rows[0] is None or not rows:
if not rows or not isinstance(rows, list) or not rows[0]:

f" ALTER SESSION SET CURRENT_SCHEMA = :db_name ",
{"db_name": db_name},
)
cursor.execute(f' ALTER SESSION SET CURRENT_SCHEMA = "{db_name}" ')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是为了防止注入, 请回滚

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果低版本的不支持, 还请麻烦提供其他防止sql注入的手段

@taijiyouxia taijiyouxia closed this by deleting the head repository Jul 24, 2023
@tonyhu214
Copy link
Contributor

@LeoQuote 我们可以使用connection.current_schema=db_name代替 cursor.execute(f' ALTER SESSION SET CURRENT_SCHEMA = "{db_name}" ') 参考https://cx-oracle.readthedocs.io/en/latest/api_manual/connection.html?highlight=CURRENT_SCHEMA#Connection.current_schema 官方文档

@LeoQuote
Copy link
Collaborator

@tonyhu214 如果知道方案的话, 还请麻烦提个 pr, 多谢了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants