[NFC] Merge report path tables in the database by bruntib · Pull Request #4946 · Ericsson/codechecker

bruntib · 2026-07-03T09:41:44Z

There are 3 database tables for storing information about the report paths:

BugPathEvent
BugReportPoint
ExtendedReportData

These are now merged to ReportPathData.

These tabels were the biggest ones in terms of database size and their handling and querying was problematic. The new table contains all report path related information as a zip blob. Keeping track of the files touched by the report path is still important, so a join-table is storing this information.

gulyasgergely902

LGTM

gulyasgergely902 · 2026-08-14T07:56:00Z

+                used_file_ids.add(file_path_to_id[macro.file.path])
+
+            report_path_data = ReportPathData(db_report.id, path_data)
+            # TODO: Here we query the File objects with session.get() that runs


Should these TODOs be in the code? I'd instead create a ticket for this and add it to the sprint.

barnabasdomozi · 2026-08-14T16:59:25Z

-    def __add_report_context(self, session, file_path_to_id):
+    def __add_report_context(
+        self,
+        session: DBSession,


The correct type annonation for session is SA_Session not DBSession.

Are you sure about it? When I follow back the type of session through the function call chain, it ends up in a DBSession.

barnabasdomozi · 2026-08-14T17:35:53Z

-                    idx, file_path_to_id[path_pos.file.path], db_report.id))
+            for path_pos in report.bug_path_positions:
+                path_data.append({
+                    "from": {


While this works, using a raw dictionary for structured data is prone to errors.
Just to name a few looking at this example:

Making a typo in from e.g. frmo, and appending that to path_data.

Not following the structure correctly, e.g. placing row outside from.

Entirely missing a field, such as appending an element that has no fid

These are all mistakes that are easy to make but hard to notice.
In contrast, consider putting all these information into a Python dataclass which are designed for these usecases.
With a dataclass, all of the fields are required params, and a type checker can also verify that a dataclass is properly constructed. Type checkers usually treat raw dictionaries as unstructured data and don't perform checks on them.

This comment applies especially to report_server.py where this data retrieved.

A raw dictionary can be an internal representation of the data, which is eventually stored (and compressed) in the database. But in my opinion, developers should access it via a type safe interface.

barnabasdomozi · 2026-08-14T17:51:56Z

+            # session object. We should investigate whether it's possible to
+            # provide a session object that has the File objects already, in
+            # order to save extra query time.
+            report_path_data.files.extend(


Running individual SELECT statements for all the files seems like a performance regression to me. And also considering that this code runs for every report during a store.

The questions are:

Why do we need to select the individual File here associated with a file_id? Why not just insert the file_id into the database?

Would that be an option to perform a JOIN between the two tables and not individual SELECT statements?

barnabasdomozi · 2026-08-14T17:54:13Z


 def get_reports_by_bugpath_filter_for_single_origin(
-    session,
+    session: DBSession,


I think this type annotation for session is wrong here as well.

barnabasdomozi · 2026-08-14T18:00:09Z

+                   initially="DEFERRED",
+                   ondelete="CASCADE"),
+        index=True),
+    Column('file_id', Integer, ForeignKey('files.id'))


On delete CASCADE was not added here, is that intentional? If a File is removed, shouldn't this row be removed as well?

barnabasdomozi · 2026-08-14T18:14:45Z

+        .filter(ReportPathData.report_id.in_(report_ids))
+
+    for rpd in report_path_data:
+        files = {f.id: f.filepath for f in rpd.files}


In this case, I'm not sure how SQLAlchemy retrieves the File data from the database.

Does it perform an individual SELECT for each file_id? If so, this looks like a performance concern to me.

Or a JOIN between the tables?

This is worth considering because this code runs in a loop.
Another follow up question: does it happen that a single File is queried multiple times for the same file_id since this code is running in a loop?

Actually, the query is running from the report's point of view. This query selects the set of files belonging to a given report. This happens with a single select statement.

I'm concerned about the performance of the current implementation.
Can't we query the required Files once instead of individual SELECTs for each report_path_data?

E.g. a proposed solution is to move files = {..} above the for loop.

barnabasdomozi

Please also fix the failing test cases.

barnabasdomozi · 2026-08-18T15:27:33Z

+        type: str
+        msg: Optional[str] = None
+
+        def to_dict(self):


Consider using https://docs.python.org/3/library/dataclasses.html#dataclasses.asdict

barnabasdomozi · 2026-08-18T15:28:35Z

-    col_begin = Column(Integer)
-    line_end = Column(Integer)
-    col_end = Column(Integer)
+    @hybrid_property


Why use @hybrid_property here?
Can't we use a regular @property instead?

barnabasdomozi · 2026-08-18T15:32:28Z

+        .filter(ReportPathData.report_id.in_(report_ids))
+
+    for rpd in report_path_data:
+        files = {f.id: f.filepath for f in rpd.files}


I'm concerned about the performance of the current implementation.
Can't we query the required Files once instead of individual SELECTs for each report_path_data?

E.g. a proposed solution is to move files = {..} above the for loop.

barnabasdomozi · 2026-08-18T15:34:06Z

+            }
+
+        @classmethod
+        def from_dict(cls, data: dict):


There is also a built-in way to construct a dataclass from a dict.

barnabasdomozi · 2026-08-18T15:35:22Z

-                    macro.range.end_line, macro.range.end_col,
-                    macro.message, file_path_to_id[macro.file.path],
-                    db_report.id, data_type))
+            rpdf = ReportPathDataFile.insert().values([{


Is this running multiple INSERT statements or only one and inserts in bulk?

barnabasdomozi · 2026-08-19T14:13:27Z

-
-def get_reports_by_bugpath_filter(session, file_filter_q) -> Set[int]:
+def get_reports_by_bugpath_filter(
+    session: DBSession,


session should by typed SA_Session here as well.

barnabasdomozi · 2026-08-19T14:21:07Z

-        endCol=brp.col_end,
+def bugreportpoint_db_to_api(brp: ReportPathData.Item) -> ttypes.BugPathPos:
+    return ttypes.BugPathPos(
+        startLine=brp.to_row,


I think startLine should be from_row and not to_row.
to_row is also used for endLine.

barnabasdomozi · 2026-08-19T14:25:12Z

                self.__validate_and_add_report_annotations(
                    session, db_report.id, report.annotations)

+        rpdf = ReportPathDataFile.insert().values(path_data_files)


If path_data_files is any empty list in the end, this INSERT should be skipped.

barnabasdomozi · 2026-08-19T14:26:37Z


-def get_report_details(session, report_ids):
+def get_report_details(
+    session: DBSession,


session should by typed SA_Session here as well.

barnabasdomozi · 2026-08-24T11:16:47Z

+        sa.ForeignKeyConstraint(
+            ['file_id'],
+            ['files.id'],
+            name=op.f('fk_report_path_data_files_file_id_files')),


file_id has no ondelete=CASCADE constraints as defined in the model file.

barnabasdomozi · 2026-08-24T11:21:39Z


-    def __init__(self, line_begin, col_begin, line_end, col_end,
-                 message, file_id, report_id, data_type):
+    files = relationship("File", secondary=ReportPathDataFile)


get_report_details() no longer relies on calling this .files relationship anymore. Is this relationship still needed or can be removed?

barnabasdomozi · 2026-08-24T11:36:15Z

                    session, db_report.id, report.annotations)

+        if path_data_files:
+            rpdf = ReportPathDataFile.insert().values(path_data_files)


Consider:

session.execute(ReportPathDataFile.insert(), path_data_files)

This way, SQLAlchemy still inserts the data in large batches but keeps it under maximum insertion limits enforced by PostgreSQL / SQLite.

barnabasdomozi

I just did one final step and tested the migration locally on an SQLite database.
When this schema migration is executed together in chain with another migration, it results in the following error:

Traceback (most recent call last):
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1969, in _exec_single_context
    self.dialect.do_execute(
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/default.py", line 952, in do_execute
    cursor.execute(statement, parameters)
sqlite3.OperationalError: cannot start a transaction within a transaction

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/workspace/github/ericsson-codechecker/build/CodeChecker/lib/python3/codechecker_server/database/database.py", line 334, in upgrade
    command.upgrade(cfg, "head")
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/alembic/command.py", line 487, in upgrade
    script.run_env()
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/alembic/script/base.py", line 550, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/alembic/util/pyfiles.py", line 114, in load_python_file
    module = load_module_py(module_id, path)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/alembic/util/pyfiles.py", line 132, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap_external>", line 995, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/workspace/github/ericsson-codechecker/build/CodeChecker/lib/python3/codechecker_server/migrations/report/env.py", line 66, in <module>
    run_migrations_online()
  File "/workspace/github/ericsson-codechecker/build/CodeChecker/lib/python3/codechecker_server/migrations/report/env.py", line 53, in run_migrations_online
    migrate(connection)
  File "/workspace/github/ericsson-codechecker/build/CodeChecker/lib/python3/codechecker_server/migrations/report/env.py", line 49, in migrate
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/alembic/runtime/environment.py", line 967, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/alembic/runtime/migration.py", line 621, in run_migrations
    step.migration_fn(**kw)
  File "/workspace/github/ericsson-codechecker/web/server/codechecker_server/migrations/report/versions/6c3c93a826f4_report_path_data_table.py", line 77, in upgrade
    conn.execute(sa.text("BEGIN"))
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1421, in execute
    return meth(
           ^^^^^
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/sql/elements.py", line 526, in _execute_on_connection
    return connection._execute_clauseelement(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1643, in _execute_clauseelement
    ret = self._execute_context(
          ^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1848, in _execute_context
    return self._exec_single_context(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1988, in _exec_single_context
    self._handle_dbapi_exception(
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 2365, in _handle_dbapi_exception
    raise sqlalchemy_exception.with_traceback(exc_info[2]) from e
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1969, in _exec_single_context
    self.dialect.do_execute(
  File "/workspace/github/codechecker/venv_dev/lib/python3.12/site-packages/sqlalchemy/engine/default.py", line 952, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) cannot start a transaction within a transaction
[SQL: BEGIN]
(Background on this error at: https://sqlalche.me/e/20/e3q8)
[INFO 2026-08-25 14:52:15] - [Default] Done upgrading. Schema upgrade failed.

There are 3 database tables for storing information about the report paths: - BugPathEvent - BugReportPoint - ExtendedReportData These are now merged to ReportPathData. These tabels were the biggest ones in terms of database size and their handling and querying was problematic. The new table contains all report path related information as a zip blob. Keeping track of the files touched by the report path is still important, so a join-table is storing this information.

barnabasdomozi

LGTM, great work

bruntib · 2026-08-25T21:03:50Z

Thank you for the extensive review and the useful comments!

bruntib added this to the release 6.29.0 milestone Jul 3, 2026

bruntib requested a review from barnabasdomozi July 3, 2026 09:41

bruntib requested a review from vodorok as a code owner July 3, 2026 09:41

bruntib force-pushed the merge_bug_path_tables branch 4 times, most recently from 4417c95 to 7e39437 Compare July 6, 2026 07:17

bruntib marked this pull request as draft July 6, 2026 09:09

This was referenced Jul 26, 2026

sqlite3.IntegrityError #4138

Open

[web] Individual Bug Path Events should not be stored as separate records #4268

Open

bruntib force-pushed the merge_bug_path_tables branch from 7e39437 to a5ea08b Compare August 13, 2026 09:14

bruntib marked this pull request as ready for review August 13, 2026 09:15

bruntib force-pushed the merge_bug_path_tables branch 2 times, most recently from a75e0f6 to 8c963fd Compare August 13, 2026 14:44

gulyasgergely902 approved these changes Aug 14, 2026

View reviewed changes

barnabasdomozi requested changes Aug 14, 2026

View reviewed changes

bruntib force-pushed the merge_bug_path_tables branch from 8c963fd to 693fef0 Compare August 17, 2026 13:52

bruntib requested a review from barnabasdomozi August 17, 2026 13:53

barnabasdomozi requested changes Aug 18, 2026

View reviewed changes

bruntib force-pushed the merge_bug_path_tables branch 2 times, most recently from a074a50 to 3ce57bf Compare August 19, 2026 11:37

bruntib requested a review from barnabasdomozi August 19, 2026 11:40

barnabasdomozi requested changes Aug 19, 2026

View reviewed changes

bruntib force-pushed the merge_bug_path_tables branch from 3ce57bf to ba0724d Compare August 24, 2026 08:06

bruntib requested a review from barnabasdomozi August 24, 2026 08:07

barnabasdomozi requested changes Aug 24, 2026

View reviewed changes

bruntib force-pushed the merge_bug_path_tables branch from ba0724d to 2f764a4 Compare August 24, 2026 15:18

bruntib requested a review from barnabasdomozi August 24, 2026 15:19

barnabasdomozi requested changes Aug 25, 2026

View reviewed changes

bruntib force-pushed the merge_bug_path_tables branch from 2f764a4 to 0dd14c2 Compare August 25, 2026 14:09

bruntib force-pushed the merge_bug_path_tables branch from e392f09 to d25a4b3 Compare August 25, 2026 17:51

barnabasdomozi reviewed Aug 25, 2026

View reviewed changes

Comment thread web/server/codechecker_server/database/run_db_model.py Outdated

bruntib force-pushed the merge_bug_path_tables branch from d25a4b3 to 554ad78 Compare August 25, 2026 18:20

bruntib added 2 commits August 25, 2026 20:20

Rename a local variable so it follows the naming conventions

554ad78

barnabasdomozi self-requested a review August 25, 2026 18:37

barnabasdomozi approved these changes Aug 25, 2026

View reviewed changes

bruntib merged commit 1aa2a10 into Ericsson:master Aug 25, 2026
15 of 19 checks passed

bruntib deleted the merge_bug_path_tables branch August 25, 2026 21:07

Uh oh!

Conversation

bruntib commented Jul 3, 2026

Uh oh!

gulyasgergely902 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barnabasdomozi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barnabasdomozi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

barnabasdomozi left a comment

Choose a reason for hiding this comment

Uh oh!

bruntib commented Aug 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants