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.

Sign up for GitHub

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

Jump to bottom

修复mongodb不能执行长sql的问题 #1556

Merged
merged 5 commits into from
Jun 3, 2022
Merged

修复mongodb不能执行长sql的问题 #1556

merged 5 commits into from
Jun 3, 2022

Conversation

czxin788
Copy link
Contributor

@czxin788 czxin788 commented May 30, 2022 •
edited by LeoQuote
Loading

问题详情见：
fix: #1552

@codecov
Copy link

codecov bot commented May 30, 2022 •
edited
Loading

Codecov Report

Merging #1556 (c9f16f5) into master (3aff65d) will decrease coverage by 0.05%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##           master    #1556      +/-   ##
==========================================
- Coverage   76.84%   76.79%   -0.06%     
==========================================
  Files          91       91              
  Lines       14309    14320      +11     
==========================================
+ Hits        10996    10997       +1     
- Misses       3313     3323      +10
Impacted Files Coverage Δ
sql/engines/mongo.py 47.58% <7.69%> (-0.66%) ⬇️

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 3aff65d...c9f16f5. Read the comment docs.

@czxin788 czxin788 mentioned this pull request May 30, 2022
Closed
LeoQuote
LeoQuote reviewed May 30, 2022
View reviewed changes
sql/engines/mongo.py Outdated Show resolved Hide resolved
LeoQuote
LeoQuote reviewed May 30, 2022
View reviewed changes
sql/engines/mongo.py Outdated
else:
cmd = "{mongo} --quiet -u {user} -p '{password}' {host}:{port}/{auth_db} <<\\EOF\nrs.slaveOk();{sql}\nEOF".format(
mongo=mongo, user=self.user, password=self.password, host=self.host, port=self.port, db_name=db_name, sql=sql, auth_db=auth_db)
logger.debug(cmd)
Copy link
Collaborator

Choose a reason for hiding this comment

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

代码扫描说这个有可能把密码打印出来, 不过应该还好哈

LeoQuote
LeoQuote previously approved these changes May 30, 2022
View reviewed changes
Copy link
Collaborator

@LeoQuote LeoQuote left a comment

Choose a reason for hiding this comment

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

LGTM

@czxin788
Copy link
Contributor Author

刚有人提醒，这么改后，执行命令后的结果看不到了，我这边测试确实如此，但是还不会解决。
修改前可以正常显示执行结果：
image

修改后，就没有了：
image

@nick2wang
Copy link
Collaborator

刚有人提醒，这么改后，执行命令后的结果看不到了，我这边测试确实如此，但是还不会解决。 修改前可以正常显示执行结果： image

修改后，就没有了： image

不要用load()，直接命令行执行去获取返回

mongo  --quiet ...... < /tmp/abc.js

@czxin788
Copy link
Contributor Author

czxin788 commented May 31, 2022 •
edited
Loading

现在显示执行结果的信息了，就是结果有的有点长
image

image

@czxin788
Copy link
Contributor Author

czxin788 commented Jun 1, 2022

@fancy-lee 已经改了，你看看行吗。如果有不合理的地方，麻烦帮忙指出或修改
sql长度大于4000个字符时，使用load方法；小于4000个字符，直接执行，减少磁盘交换

@fancy-lee
Copy link
Contributor

@fancy-lee 已经改了，你看看行吗。如果有不合理的地方，麻烦帮忙指出或修改 sql长度大于4000个字符时，使用load方法；小于4000个字符，直接执行，减少磁盘交换

目测没有问题

@hhyo hhyo merged commit 872c551 into hhyo:master Jun 3, 2022
@fancy-lee
Copy link
Contributor

fancy-lee commented May 17, 2023 •
edited
Loading

@czxin788 这里又引出了来个新的bug，也可能是后面加执行影响行数的代码没有测试到，就是用load执行完了,返回结果默认会加一个true
image
由于在返回执行结果的时间没有对true做处理，所以json.load会报错
image
解决这个问题的办法是：
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fancy-lee fancy-lee fancy-lee approved these changes

@LeoQuote LeoQuote LeoQuote left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ bug ]如果sql语句过长，mongodb 执行sql会不成功，我已经复现
5 participants
@czxin788 @nick2wang @fancy-lee @LeoQuote @hhyo