-
Notifications
You must be signed in to change notification settings - Fork 4
ctk info table-traffic
#528
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,192 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import dataclasses | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import logging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import List, Any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import polars as pl | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import attr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import sqlparse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from boltons.iterutils import flatten | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sqlparse.tokens import Keyword | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from cratedb_toolkit.util.database import DatabaseAdapter, get_table_names | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@attr.define | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class Operation: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
op: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stmt: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tables_symbols: List[str] = attr.field(factory=list) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# tables_effective: List[str] = attr.field(factory=list) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@attr.define | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class Operations: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
data: List[Operation] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def foo(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fj = [attr.asdict(j) for j in self.data] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
df = pl.from_records(fj) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
print(df) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#grouped = df.group_by("tables_symbols").agg([pl.sum("tables_symbols"), pl.sum("op")]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
grouped = df.sql("SELECT tables_symbols, COUNT(op) FROM self GROUP BY tables_symbols") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
print(grouped) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+27
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Replace prints and fragile DataFrame SQL with a deterministic aggregation API.
Refactor to explode the list column, then group and count, and return a DataFrame. Apply this diff: - def foo(self):
- fj = [attr.asdict(j) for j in self.data]
- df = pl.from_records(fj)
- print(df)
- #grouped = df.group_by("tables_symbols").agg([pl.sum("tables_symbols"), pl.sum("op")])
- grouped = df.sql("SELECT tables_symbols, COUNT(op) FROM self GROUP BY tables_symbols")
- print(grouped)
+ def summarize_by_table(self) -> pl.DataFrame:
+ fj = [attr.asdict(j) for j in self.data]
+ if not fj:
+ return pl.DataFrame({"tables_symbols": [], "count": []})
+ df = pl.from_records(fj)
+ # Count statements per table (explode list column first, then group).
+ df = df.explode("tables_symbols")
+ return (
+ df.group_by("tables_symbols")
+ .agg(pl.len().alias("count"))
+ .sort("count", descending=True)
+ ) 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.12.2)30-30: Remove (T201) 31-31: Found commented-out code Remove commented-out code (ERA001) 33-33: Remove (T201) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class TableTraffic: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def __init__(self, adapter: DatabaseAdapter): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.adapter = adapter | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+36
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Pass scrub via constructor for consistent PII handling. Expose Apply this diff: -class TableTraffic:
-
- def __init__(self, adapter: DatabaseAdapter):
- self.adapter = adapter
+class TableTraffic:
+
+ def __init__(self, adapter: DatabaseAdapter, scrub: bool = False):
+ self.adapter = adapter
+ self.scrub = scrub 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def read_jobs_database(self, begin: int = 0, end: int = 0): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Reading sys.jobs_log") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
now = int(time.time() * 1000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
end = end or now | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
begin = begin or now - 600 * 60 * 1000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stmt = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"SELECT " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"started, ended, classification, stmt, username, node " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"FROM sys.jobs_log " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"WHERE " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"stmt NOT LIKE '%sys.%' AND " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"stmt NOT LIKE '%information_schema.%' " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"AND ended BETWEEN {begin} AND {end} " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"ORDER BY ended ASC" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return self.adapter.run_sql(stmt, records=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def read_jobs(self, jobs) -> List[Operation]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for job in jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sql = job["stmt"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result.append(self.parse_expression(sql)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+58
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Redact raw SQL when scrubbing and prepare data for aggregation. Use Apply this diff: def read_jobs(self, jobs) -> List[Operation]:
result = []
for job in jobs:
sql = job["stmt"]
- result.append(self.parse_expression(sql))
+ op = self.parse_expression(sql)
+ if self.scrub:
+ op.stmt = None # redact raw SQL text
+ result.append(op)
return result 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def parse_expression(sql: str) -> Operation: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.debug(f"Analyzing SQL: {sql}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
classifier = SqlStatementClassifier(expression=sql) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not classifier.operation: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.warning(f"Unable to determine operation: {sql}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not classifier.table_names: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.warning(f"Unable to determine table names: {sql}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Operation( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
op=classifier.operation, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stmt=sql, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tables_symbols=classifier.table_names, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+65
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Harden parsing: default unknown op, deduplicate table names. Avoid Apply this diff: @staticmethod
def parse_expression(sql: str) -> Operation:
logger.debug(f"Analyzing SQL: {sql}")
classifier = SqlStatementClassifier(expression=sql)
if not classifier.operation:
logger.warning(f"Unable to determine operation: {sql}")
if not classifier.table_names:
logger.warning(f"Unable to determine table names: {sql}")
- return Operation(
- op=classifier.operation,
+ table_names = list(dict.fromkeys(classifier.table_names))
+ return Operation(
+ op=classifier.operation or "UNKNOWN",
stmt=sql,
- tables_symbols=classifier.table_names,
+ tables_symbols=table_names,
) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def analyze_jobs(self, ops: Operations): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ops.foo() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def render(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jobs = self.read_jobs_database() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info(f"Analyzing {len(jobs)} jobs") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ops = Operations(self.read_jobs(jobs)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jobsa = self.analyze_jobs(ops) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info(f"Result: {jobsa}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+79
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Make analyze/render return structured results; avoid side-effect-only logging. Currently Apply this diff: - def analyze_jobs(self, ops: Operations):
- ops.foo()
+ def analyze_jobs(self, ops: Operations) -> pl.DataFrame:
+ return ops.summarize_by_table()
- def render(self):
+ def render(self):
jobs = self.read_jobs_database()
logger.info(f"Analyzing {len(jobs)} jobs")
- ops = Operations(self.read_jobs(jobs))
- jobsa = self.analyze_jobs(ops)
- logger.info(f"Result: {jobsa}")
+ ops = Operations(self.read_jobs(jobs))
+ summary_df = self.analyze_jobs(ops)
+ return summary_df.to_dicts() 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@dataclasses.dataclass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class SqlStatementClassifier: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Helper to classify an SQL statement. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Here, most importantly: Provide the `is_dql` property that | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
signals truthfulness for read-only SQL SELECT statements only. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expression: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
permit_all: bool = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_parsed_sqlparse: Any = dataclasses.field(init=False, default=None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def __post_init__(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if self.expression is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.expression = "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if self.expression: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.expression = self.expression.strip() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def parse_sqlparse(self) -> List[sqlparse.sql.Statement]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Parse expression using traditional `sqlparse` library. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if self._parsed_sqlparse is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self._parsed_sqlparse = sqlparse.parse(self.expression) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return self._parsed_sqlparse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def is_dql(self) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Is it a DQL statement, which effectively invokes read-only operations only? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not self.expression: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if self.permit_all: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Check if the expression is valid and if it's a DQL/SELECT statement, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# also trying to consider `SELECT ... INTO ...` and evasive | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# `SELECT * FROM users; \uff1b DROP TABLE users` statements. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return self.is_select and not self.is_camouflage | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def is_select(self) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Whether the expression is an SQL SELECT statement. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return self.operation == "SELECT" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def operation(self) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The SQL operation: SELECT, INSERT, UPDATE, DELETE, CREATE, etc. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
parsed = self.parse_sqlparse() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return parsed[0].get_type().upper() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+143
to
+149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard against empty parses in
Apply this diff: @property
def operation(self) -> str:
"""
The SQL operation: SELECT, INSERT, UPDATE, DELETE, CREATE, etc.
"""
- parsed = self.parse_sqlparse()
- return parsed[0].get_type().upper()
+ parsed = self.parse_sqlparse()
+ if not parsed:
+ return ""
+ return (parsed[0].get_type() or "").upper() 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def table_names(self) -> List[str]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The SQL operation: SELECT, INSERT, UPDATE, DELETE, CREATE, etc. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return flatten(get_table_names(self.expression)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def is_camouflage(self) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Innocent-looking `SELECT` statements can evade filters. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return self.is_select_into or self.is_evasive | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def is_select_into(self) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Use traditional `sqlparse` for catching `SELECT ... INTO ...` statements. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Examples: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SELECT * INTO foobar FROM bazqux | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SELECT * FROM bazqux INTO foobar | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Flatten all tokens (including nested ones) and match on type+value. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
statement = self.parse_sqlparse()[0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return any( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
token.ttype is Keyword and token.value.upper() == "INTO" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for token in statement.flatten() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+165
to
+177
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Guard Prevent potential Apply this diff: # Flatten all tokens (including nested ones) and match on type+value.
- statement = self.parse_sqlparse()[0]
+ parsed = self.parse_sqlparse()
+ if not parsed:
+ return False
+ statement = parsed[0]
return any(
token.ttype is Keyword and token.value.upper() == "INTO"
for token in statement.flatten()
) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def is_evasive(self) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Use traditional `sqlparse` for catching evasive SQL statements. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
A practice picked up from CodeRabbit was to reject multiple statements | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
to prevent potential SQL injections. Is it a viable suggestion? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Examples: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SELECT * FROM users; \uff1b DROP TABLE users | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
parsed = self.parse_sqlparse() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return len(parsed) > 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Wire scrub, return structured data, and emit JSON like other info commands.
scrub
is read but unused (Ruff F841).TableTraffic.render()
currently logs and prints in its implementation; the CLI should output structured JSON like the other commands.Refactor to pass
scrub
, return data, and calljd()
.Apply this diff:
📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.12.2)
95-95: Local variable
scrub
is assigned to but never usedRemove assignment to unused variable
scrub
(F841)
🤖 Prompt for AI Agents