tp: rework handling of implicit and parent columns
Mutability of Table leads to very subtle bugs which are hard to track
down. Instead, have wrappers for Table and Column which are themselves
immutable and handle implicit and parent columns there.
Change-Id: I8363974af63acece9734e422e8364d1b3ce23f82
diff --git a/tools/gen_tp_table_docs.py b/tools/gen_tp_table_docs.py
index 583f339..f618bc0 100755
--- a/tools/gen_tp_table_docs.py
+++ b/tools/gen_tp_table_docs.py
@@ -16,73 +16,69 @@
import argparse
import json
import os
-import runpy
import sys
from typing import Any
from typing import Dict
-from typing import List
from typing import Optional
-from typing import Union
# Allow importing of root-relative modules.
ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
sys.path.append(os.path.join(ROOT_DIR))
-from python.generators.trace_processor_table.public import Column
+#pylint: disable=wrong-import-position
from python.generators.trace_processor_table.public import ColumnDoc
-from python.generators.trace_processor_table.public import Table
import python.generators.trace_processor_table.util as util
+from python.generators.trace_processor_table.util import ParsedTable
+from python.generators.trace_processor_table.util import ParsedColumn
+#pylint: enable=wrong-import-position
-def gen_json_for_column(table: Table, col: Column,
- doc: Union[ColumnDoc, str]) -> Optional[Dict[str, Any]]:
+def gen_json_for_column(table: ParsedTable,
+ col: ParsedColumn) -> Optional[Dict[str, Any]]:
"""Generates the JSON documentation for a column in a table."""
# id and type columns should be skipped if the table specifies so.
- is_skippable_col = col._is_auto_added_id or col._is_auto_added_type
- if table.tabledoc.skip_id_and_type and is_skippable_col:
+ is_skippable_col = col.is_implicit_id or col.is_implicit_type
+ if table.table.tabledoc.skip_id_and_type and is_skippable_col:
return None
# Our default assumption is the documentation for a column is a plain string
# so just make the comment for the column equal to that.
- if isinstance(doc, ColumnDoc):
- comment = doc.doc
- if doc.joinable:
- join_table, join_type = doc.joinable.split('.')
+ if isinstance(col.doc, ColumnDoc):
+ comment = col.doc.doc
+ if col.doc.joinable:
+ join_table, join_col = col.doc.joinable.split('.')
else:
- join_table, join_type = None, None
- elif isinstance(doc, str):
- comment = doc
- join_table, join_type = None, None
+ join_table, join_col = None, None
+ elif isinstance(col.doc, str):
+ comment = col.doc
+ join_table, join_col = None, None
else:
raise Exception('Unknown column documentation type')
- parsed_type = util.parse_type(table, col.type)
+ parsed_type = table.parse_type(col.column.type)
docs_type = parsed_type.cpp_type
if docs_type == 'StringPool::Id':
docs_type = 'string'
ref_class_name = None
- if parsed_type.id_table and not col._is_auto_added_id:
- id_table_name = util.public_sql_name_for_table(parsed_type.id_table)
+ if parsed_type.id_table and not col.is_implicit_id:
+ id_table_name = util.public_sql_name(parsed_type.id_table)
ref_class_name = parsed_type.id_table.class_name
- # We shouldn't really specify the join tables when it's a simple id join.
- assert join_table is None
- assert join_type is None
-
- join_table = id_table_name
- join_type = "id"
+ if not join_table and not join_col:
+ join_table = id_table_name
+ join_col = "id"
return {
- 'name': col.name,
+ 'name': col.column.name,
'type': docs_type,
'comment': comment,
'optional': parsed_type.is_optional,
'refTableCppName': ref_class_name,
'joinTable': join_table,
- 'joinCol': join_type,
+ 'joinCol': join_col,
}
@@ -92,21 +88,17 @@
parser.add_argument('inputs', nargs='*')
args = parser.parse_args()
- tables: List[Table] = []
- for in_path in args.inputs:
- tables.extend(runpy.run_path(in_path)['ALL_TABLES'])
- for table in tables:
- util.normalize_table_columns(table)
-
+ tables = util.parse_tables_from_files(args.inputs)
table_docs = []
- for table in tables:
+ for parsed in tables:
+ table = parsed.table
doc = table.tabledoc
cols = (
- gen_json_for_column(table, c, doc.columns[c.name])
- for c in table.columns
- if c._is_self_column)
+ gen_json_for_column(parsed, c)
+ for c in parsed.columns
+ if not c.is_ancestor)
table_docs.append({
- 'name': util.public_sql_name_for_table(table),
+ 'name': util.public_sql_name(table),
'cppClassName': table.class_name,
'defMacro': table.class_name,
'comment': '\n'.join(l.strip() for l in doc.doc.splitlines()),
diff --git a/tools/gen_tp_table_headers.py b/tools/gen_tp_table_headers.py
index 2f4f8c4..a039e3c 100755
--- a/tools/gen_tp_table_headers.py
+++ b/tools/gen_tp_table_headers.py
@@ -17,8 +17,8 @@
from dataclasses import dataclass
import os
import re
-import runpy
import sys
+from typing import Dict
from typing import List
from typing import Set
@@ -27,12 +27,9 @@
sys.path.append(os.path.join(ROOT_DIR))
#pylint: disable=wrong-import-position
-from python.generators.trace_processor_table.public import Alias
-from python.generators.trace_processor_table.public import Table
from python.generators.trace_processor_table.serialize import serialize_header
-from python.generators.trace_processor_table.util import find_table_deps
-from python.generators.trace_processor_table.util import normalize_table_columns
-from python.generators.trace_processor_table.util import topological_sort_tables
+from python.generators.trace_processor_table.util import ParsedTable
+from python.generators.trace_processor_table.util import parse_tables_from_files
#pylint: enable=wrong-import-position
@@ -41,7 +38,7 @@
"""Represents a Python module which will be converted to a header."""
out_path: str
relout_path: str
- tables: List[Table]
+ tables: List[ParsedTable]
def main():
@@ -55,49 +52,38 @@
if len(args.inputs) != len(args.outputs):
raise Exception('Number of inputs must match number of outputs')
- headers: List[Header] = []
- for (in_path, out_path) in zip(args.inputs, args.outputs):
- tables = runpy.run_path(in_path)['ALL_TABLES']
+ in_to_out = dict(zip(args.inputs, args.outputs))
+ headers: Dict[str, Header] = {}
+ for table in parse_tables_from_files(args.inputs):
+ out_path = in_to_out[table.input_path]
relout_path = os.path.relpath(out_path, args.gen_dir)
- headers.append(Header(out_path, relout_path, tables))
+
+ header = headers.get(table.input_path, Header(out_path, relout_path, []))
+ header.tables.append(table)
+ headers[table.input_path] = header
# Build a mapping from table class name to the output path of the header
# which will be generated for it. This is used to include one header into
# another for Id dependencies.
table_class_name_to_relout = {}
- for header in headers:
+ for header in headers.values():
for table in header.tables:
- table_class_name_to_relout[table.class_name] = header.relout_path
+ table_class_name_to_relout[table.table.class_name] = header.relout_path
- for header in headers:
- # Remove any alias columns. Today these are handled in SQL not C++: this may
- # change in the future.
- for table in header.tables:
- table.columns = [
- c for c in table.columns if not isinstance(c.type, Alias)
- ]
-
- # Topologically sort the tables in this header to ensure that any deps are
- # defined *before* the table itself.
- sorted_tables = topological_sort_tables(header.tables)
-
- # Normalize all the columns to ensure that they are ready for serialization.
- for table in sorted_tables:
- normalize_table_columns(table)
-
+ for header in headers.values():
# Find all headers depended on by this table. These will be #include-ed when
# generating the header file below so ensure we remove ourself.
header_relout_deps: Set[str] = set()
- for table in sorted_tables:
+ for table in header.tables:
header_relout_deps.union(
- table_class_name_to_relout[c] for c in find_table_deps(table))
+ table_class_name_to_relout[c] for c in table.find_table_deps())
header_relout_deps.discard(header.relout_path)
with open(header.out_path, 'w', encoding='utf8') as out:
ifdef_guard = re.sub(r'[^a-zA-Z0-9_-]', '_',
header.relout_path).upper() + '_'
out.write(
- serialize_header(ifdef_guard, sorted_tables,
+ serialize_header(ifdef_guard, header.tables,
sorted(header_relout_deps)))
out.write('\n')
