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

Add query text sample & wrap ? to one #1010

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
@seanlook
  1. Add query_text column to stats_mysql_query_digest to log the whole sql text sample for sql preview or developer's usage as descripted in topic Add query sample for digest.
    Known issue: in some random situation, query text will contaion unexpected characters at its tail:
*************************** 22. row ***************************
  hostgroup: 110
 schemaname: information_schema
   username: xxxx
     digest: 0x1940FED1ABE8D6BC
digest_text: SHOW KEYS FROM `t_mytest1` FROM `mydb`
 count_star: 1
 first_seen: 1493294924
  last_seen: 1493294924
   sum_time: 912
   min_time: 912
   max_time: 912
 query_text: SHOW KEYS FROM `t_mytest1` FROM `mydb`m    <=

Or like this:
insert into user (id, name, pwd\n      )\n    values (null, 'zzz', '3333'\n      )Î^?    <=
  1. Wrap two more question mark(?) to one (?,), like
select * from t where id in (1, 2, 3)
↓
select * from t where id in (?,)

There is also some exceptions in c_tokenizer.c :

select * from t where f_waste_id id ( 1 ,2)
↓
select * from t where f_waste_id id ( ??)

But I really don't mind that.

zhouxiao added some commits Apr 27, 2017

zhouxiao
add query text sample for digest
add query_text columns in table stats_mysql_query_digest

@seanlook seanlook changed the title from V1.3.7 querysample digest to Add query text sample & wrap ? to one Apr 28, 2017

@renecannao

This comment has been minimized.

Show comment
Hide comment
@renecannao

renecannao Apr 30, 2017

Contributor

Hi Sean,
Thank you for the pull request.
I have some concern about implementing this feature:

  • users with stats credentials shouldn't be allowed to read query_text, as it may contain sensitive information
  • should be interesting to collect the last N (configurable) queries for a specific digest, and not just one

Thoughts?

Contributor

renecannao commented Apr 30, 2017

Hi Sean,
Thank you for the pull request.
I have some concern about implementing this feature:

  • users with stats credentials shouldn't be allowed to read query_text, as it may contain sensitive information
  • should be interesting to collect the last N (configurable) queries for a specific digest, and not just one

Thoughts?

@renecannao

This comment has been minimized.

Show comment
Hide comment
@renecannao

renecannao Apr 30, 2017

Contributor

Also, new functionalities should go in 1.4.x .
1.3.x should only include bug fixes.

Contributor

renecannao commented Apr 30, 2017

Also, new functionalities should go in 1.4.x .
1.3.x should only include bug fixes.

@seanlook

This comment has been minimized.

Show comment
Hide comment
@seanlook

seanlook May 3, 2017

Hi Rene,

  1. Maybe adding a new table for admin user to manage query_text
  2. As a DBA, we should always consider the performance impact when add a new feature. Collecting the last N queries is useless for me because of the thousands of requests. I guess logging this every queries and rotating the last N may have the performance impact.
    In fact I one query text sample is enough. It's really awesome if collecting queries according to the ratio of count_star

seanlook commented May 3, 2017

Hi Rene,

  1. Maybe adding a new table for admin user to manage query_text
  2. As a DBA, we should always consider the performance impact when add a new feature. Collecting the last N queries is useless for me because of the thousands of requests. I guess logging this every queries and rotating the last N may have the performance impact.
    In fact I one query text sample is enough. It's really awesome if collecting queries according to the ratio of count_star
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment