unstract/connectors/src/unstract/connectors/databases/psycopg_handler.py (2)

38-52: Solid handling of the PostgreSQL race condition.

The approach is correct: catching DuplicateTable, rolling back the aborted transaction, and returning early makes the operation idempotent.

Optional improvement: Consider using startswith instead of in to avoid false positives if the string appears elsewhere in the query (e.g., as a literal in an INSERT).

🔧 Suggested improvement

-        if CREATE_TABLE_IF_NOT_EXISTS in sql_query.upper():
+        if sql_query.strip().upper().startswith(CREATE_TABLE_IF_NOT_EXISTS):

---

53-66: Race condition handling is appropriate.

This correctly addresses the pg_type_typname_nsp_index UniqueViolation that can occur during concurrent table creation.

Regarding the static analysis hint at line 65: While logging.exception would include the traceback, the existing handlers in this file (lines 68, 71, 74, 77, etc.) all use logger.error. Since the exception is re-raised immediately, the traceback is preserved. Keeping logger.error maintains consistency with the rest of the file.

Same startswith suggestion applies here for line 57.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

unstract/connectors/src/unstract/connectors/databases/psycopg_handler.py (1)

18-23: Harden CREATE TABLE IF NOT EXISTS detection (comments/whitespace/modifiers).

strip().upper().startswith(...) is easy to bypass with leading SQL comments, extra whitespace/newlines, or CREATE TEMP/UNLOGGED TABLE IF NOT EXISTS .... If your SQL generator ever emits those variants, the race handling won’t trigger.

Proposed refactor (more tolerant regex)

+import re
 import logging
 from typing import Any

 from psycopg2 import errors as PsycopgError

@@
 CREATE_TABLE_IF_NOT_EXISTS = "CREATE TABLE IF NOT EXISTS"

+_CREATE_TABLE_IF_NOT_EXISTS_RE = re.compile(
+    r"^\s*(?:/\*.*?\*/\s*)*(?:--[^\n]*\n\s*)*"
+    r"CREATE\s+(?:TEMP|TEMPORARY|UNLOGGED\s+)?TABLE\s+IF\s+NOT\s+EXISTS\b",
+    re.IGNORECASE | re.DOTALL,
+)
+
 def _is_create_table_if_not_exists(sql_query: str) -> bool:
     """Check if query is a CREATE TABLE IF NOT EXISTS statement."""
-    return sql_query.strip().upper().startswith(CREATE_TABLE_IF_NOT_EXISTS)
+    return bool(_CREATE_TABLE_IF_NOT_EXISTS_RE.match(sql_query))

Please sanity-check this against the actual emitted CREATE TABLE statements in your workflows.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

unstract/connectors/src/unstract/connectors/databases/psycopg_handler.py (3)

43-56: Use logger.exception for better tracebacks when re-raising.

When logging before re-raising an exception, logger.exception automatically includes the traceback, aiding debugging.

Suggested fix

-            logger.error(f"DuplicateTable error: {e.pgerror}")
+            logger.exception(f"DuplicateTable error: {e.pgerror}")

---

62-65: The constraint is None fallback may be overly permissive.

When constraint_name is unavailable (e.g., due to psycopg2/PostgreSQL version differences), any UniqueViolation during a CREATE TABLE IF NOT EXISTS will be suppressed. While this is likely safe for pure DDL statements, verify this assumption holds for your use cases.

Consider logging the raw exception details when constraint is None to aid future debugging:

Optional: Add trace logging when constraint is unknown

             is_pg_type_race = (
                 constraint == "pg_type_typname_nsp_index" or constraint is None
             )
+            if constraint is None:
+                logger.debug(
+                    f"UniqueViolation constraint_name unavailable, assuming pg_type race. "
+                    f"Full error: {e}"
+                )

---

73-74: Use logger.exception for better tracebacks when re-raising.

Same as the DuplicateTable handler—use logger.exception to capture the full traceback.

Suggested fix

-            logger.exception(f"UniqueViolation error (constraint={constraint}): {e.pgerror}")
+            logger.exception(f"UniqueViolation error (constraint={constraint}): {e.pgerror}")

Wait, let me fix this:

-            logger.error(f"UniqueViolation error (constraint={constraint}): {e.pgerror}")
+            logger.exception(f"UniqueViolation error (constraint={constraint}): {e.pgerror}")

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting