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

修复union-bug-1360 #1371

Merged
merged 28 commits into from
Mar 5, 2022
Merged

修复union-bug-1360 #1371

merged 28 commits into from
Mar 5, 2022

Conversation

unknowissue
Copy link
Contributor

@unknowissue unknowissue commented Feb 9, 2022

fix #1360

@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #1371 (709eaf7) into master (a45dd5a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1371      +/-   ##
==========================================
+ Coverage   74.89%   74.91%   +0.01%     
==========================================
  Files          82       82              
  Lines       13181    13191      +10     
==========================================
+ Hits         9872     9882      +10     
  Misses       3309     3309              
Impacted Files Coverage Δ
sql/utils/go_data_masking.py 92.51% <100.00%> (+0.24%) ⬆️
sql/utils/tests.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a45dd5a...709eaf7. Read the comment docs.

@unknowissue
Copy link
Contributor Author

还有点问题,得看看
hanchuanchuan/goInception#426

@@ -19,6 +19,7 @@
#不修改整体逻辑,主要修改由goInception返回的结果中关键字,比如db修改为schema
def go_data_masking(instance, db_name, sql, sql_result):
"""脱敏数据"""
particular_flag=0
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
Contributor Author

Choose a reason for hiding this comment

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

嗷,我想想,
我主要考虑的是其中有union或者concat关键字时的特殊处理,想用1,2,3这样的代码来简要判断

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.

1,2,3 这样的代码不是很明显, 会忘记的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1,2,3 这样的代码不是很明显, 会忘记的

好滴,后面再修改看看,
goInception那边的树有点问题,我没懂啥意思。

@hhyo
Copy link
Owner

hhyo commented Feb 10, 2022

如果使用goinception的脱敏暂时不能达到inception的效果,可以和最开始的pr一样,做成可选,方便发布下一个release版本

@unknowissue
Copy link
Contributor Author

多等等吧,那边的同学没反馈呢。

@unknowissue
Copy link
Contributor Author

可以了
哪位同学拉取试一试
需要注意goInception 要使用 v1.2.5-23.

Copy link
Owner

@hhyo hhyo left a comment

Choose a reason for hiding this comment

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

整体使用格式化工具进行一下格式化

import logging
from pickle import TRUE
Copy link
Owner

Choose a reason for hiding this comment

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

这个引用下面好像没用

@@ -29,11 +33,19 @@ def go_data_masking(instance, db_name, sql, sql_result):
sql_result.error = '不支持该查询语句脱敏!请联系管理员'
sql_result.status = 1
return sql_result
#设置一个特殊标记,要是还有特殊关键字特殊处理,如果还有其他关键字需要特殊处理再逐步增加
elif token.ttype is Keyword and token.value.upper() in ['UNION','UNION ALL']:
Copy link
Owner

Choose a reason for hiding this comment

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

上面if的空无用可以去掉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个地方不需要保留么?
我感觉要是后面有不支持的关键字往里填就行。。。

for d in query_tree:

#print(bool([s for s in values if d['field'] in s['field'] ]))
if bool([s for s in values if d['field'] in s['field'] ]) is False:
Copy link
Owner

Choose a reason for hiding this comment

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

if not 就可以

@unknowissue
Copy link
Contributor Author

test_go_data_masking_union_support_keyword
这个ci我麻了,,,,

@nick2wang
Copy link
Collaborator

test_go_data_masking_union_support_keyword 这个ci我麻了,,,,

可以先在本地跑unit test,打印更详细的日志

@unknowissue
Copy link
Contributor Author

unknowissue commented Mar 4, 2022

操,傻叉了我,晚点改下

@hhyo hhyo merged commit a7cde53 into hhyo:master Mar 5, 2022
@unknowissue unknowissue deleted the union-bug-1360 branch March 5, 2022 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] 测试基于goInception的脱敏发现两个问题
4 participants