Skip to content

Commit

Permalink
Merge "Make CREATE_TABLE_ALLOWLIST paths relative to stdlib root" int…
Browse files Browse the repository at this point in the history
…o main
  • Loading branch information
Rasika Navarange authored and Gerrit Code Review committed Nov 10, 2023
2 parents 9c4d509 + 061e829 commit 11e588e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 24 deletions.
7 changes: 4 additions & 3 deletions python/generators/sql_processing/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,18 @@ def check_banned_create_table_as(sql: str, filename: str,

# Given SQL string check whether there is (not allowlisted) usage of
# CREATE TABLE {name} AS.
def check_banned_create_table_as(sql: str, filename: str,
def check_banned_create_table_as(sql: str, filename: str, stdlib_path: str,
allowlist: Dict[str, List[str]]) -> List[str]:
errors = []
for _, matches in match_pattern(CREATE_TABLE_AS_PATTERN, sql).items():
name = matches[0]
if filename not in allowlist:
allowlist_key = filename[len(stdlib_path):]
if allowlist_key not in allowlist:
errors.append(f"CREATE TABLE '{name}' is deprecated. "
"Use CREATE PERFETTO TABLE instead.\n"
f"Offending file: {filename}\n")
continue
if name not in allowlist[filename]:
if name not in allowlist[allowlist_key]:
errors.append(
f"Table '{name}' uses CREATE TABLE which is deprecated "
"and this table is not allowlisted. Use CREATE PERFETTO TABLE.\n"
Expand Down
20 changes: 9 additions & 11 deletions tools/check_sql_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,21 @@
from python.generators.sql_processing.utils import DROP_TABLE_VIEW_PATTERN
from python.generators.sql_processing.utils import CREATE_TABLE_VIEW_PATTERN

# Allowlist path are relative to the metrics root.
CREATE_TABLE_ALLOWLIST = {
('/src/trace_processor/metrics/sql/android'
('/android'
'/android_blocking_calls_cuj_metric.sql'): [
'android_cujs', 'relevant_binder_calls_with_names',
'android_blocking_calls_cuj_calls'
],
'/src/trace_processor/metrics/sql/android/jank/cujs.sql': [
'android_jank_cuj'
],
'/src/trace_processor/metrics/sql/chrome/gesture_flow_event.sql': [
'/android/jank/cujs.sql': ['android_jank_cuj'],
'/chrome/gesture_flow_event.sql': [
'{{prefix}}_latency_info_flow_step_filtered'
],
'/src/trace_processor/metrics/sql/chrome/gesture_jank.sql': [
'/chrome/gesture_jank.sql': [
'{{prefix}}_jank_maybe_null_prev_and_next_without_precompute'
],
'/src/trace_processor/metrics/sql/experimental/frame_times.sql': [
'DisplayCompositorPresentationEvents'
]
'/experimental/frame_times.sql': ['DisplayCompositorPresentationEvents']
}


Expand All @@ -71,7 +68,7 @@ def match_drop_view_pattern_to_dict(sql: str,
return res


def check(path: str) -> List[str]:
def check(path: str, metrics_sources: str) -> List[str]:
with open(path) as f:
sql = f.read()

Expand All @@ -82,6 +79,7 @@ def check(path: str) -> List[str]:
sql, DROP_TABLE_VIEW_PATTERN)
errors = check_banned_create_table_as(sql,
path.split(ROOT_DIR)[1],
metrics_sources.split(ROOT_DIR)[1],
CREATE_TABLE_ALLOWLIST)
errors += check_banned_create_view_as(sql, path.split(ROOT_DIR)[1])
for name, [line, type] in create_table_view_dir.items():
Expand Down Expand Up @@ -110,7 +108,7 @@ def main():
for f in files:
path = os.path.join(root, f)
if path.endswith('.sql'):
errors += check(path)
errors += check(path, metrics_sources)

if errors:
sys.stderr.write("\n".join(errors))
Expand Down
20 changes: 10 additions & 10 deletions tools/check_sql_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,27 @@
from python.generators.sql_processing.utils import check_banned_words
from python.generators.sql_processing.utils import check_banned_include_all

# Allowlist path are relative to the stdlib root.
CREATE_TABLE_ALLOWLIST = {
'/src/trace_processor/perfetto_sql/stdlib/android/binder.sql': [
'/android/binder.sql': [
'internal_oom_score', 'internal_async_binder_reply',
'internal_binder_async_txn_raw'
],
'/src/trace_processor/perfetto_sql/stdlib/android/monitor_contention.sql': [
'/android/monitor_contention.sql': [
'internal_isolated', 'android_monitor_contention_chain',
'android_monitor_contention'
],
'/src/trace_processor/perfetto_sql/stdlib/chrome/tasks.sql': [
'/chrome/tasks.sql': [
'internal_chrome_mojo_slices', 'internal_chrome_java_views',
'internal_chrome_scheduler_tasks', 'internal_chrome_tasks'
],
('/src/trace_processor/perfetto_sql/stdlib/experimental/'
('/experimental/'
'thread_executing_span.sql'): [
'internal_wakeup', 'experimental_thread_executing_span_graph',
'internal_critical_path', 'internal_wakeup_graph',
'experimental_thread_executing_span_graph'
],
'/src/trace_processor/perfetto_sql/stdlib/experimental/flat_slices.sql': [
'experimental_slice_flattened'
]
'/experimental/flat_slices.sql': ['experimental_slice_flattened']
}


Expand Down Expand Up @@ -117,9 +116,10 @@ def main():

errors += parsed.errors
errors += check_banned_words(sql, path)
errors += check_banned_create_table_as(sql,
path.split(ROOT_DIR)[1],
CREATE_TABLE_ALLOWLIST)
errors += check_banned_create_table_as(
sql,
path.split(ROOT_DIR)[1],
args.stdlib_sources.split(ROOT_DIR)[1], CREATE_TABLE_ALLOWLIST)
errors += check_banned_create_view_as(sql, path.split(ROOT_DIR)[1])
errors += check_banned_include_all(sql, path.split(ROOT_DIR)[1])

Expand Down

0 comments on commit 11e588e

Please sign in to comment.