Skip to content

fix: resolve schema-qualified table names in getFieldData() and protectIdentifiers() - #10604

Open
paulbalandan wants to merge 1 commit into
codeigniter4:developfrom
paulbalandan:field-data-qualified-table
Open

paulbalandan wants to merge 1 commit into
codeigniter4:developfrom
paulbalandan:field-data-qualified-table

Conversation

@paulbalandan

Copy link
Copy Markdown
Member

Description

getFieldData() returned no fields for a schema-qualified table name (e.g. public.jobs). Found while reviewing #10601 and #10602, whose batch casts depend on it.

  • BaseConnection::protectDotItem() did not strip the escape character before checking for the prefix, so an already protected "tenant"."db_jobs" was prefixed again. The builders pass protected table names to getFieldData(), so every driver got a mangled name.
  • Postgre and SQLSRV ignored the schema segment, and SQLite3 produced an invalid PRAGMA. MySQLi and OCI8 already handled a clean schema.table.

Unqualified names behave as before. Tests cover the protectIdentifiers() cases and getFieldData() with a qualified name on all drivers.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@carson-codeigniter4 carson-codeigniter4 Bot added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 6, 2026
private function protectDotItem(string $item, string $alias, bool $protectIdentifiers, bool $fieldExists): string
{
$parts = explode('.', $item);
$parts = array_map(fn (string $part): string => trim($part, $this->escapeChar), explode('.', $item));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With QUOTED_IDENTIFIER OFF, the SQLSRV driver sets $this->escapeChar to ['[', ']']. Passing this array as the second argument to trim() causes a TypeError when processing dotted identifiers. I verified that protectIdentifiers('jobs.id') returned [jobs].[id] before this change and now throws. We should handle the array form of escapeChar and add a regression test.

private function protectDotItem(string $item, string $alias, bool $protectIdentifiers, bool $fieldExists): string
{
$parts = explode('.', $item);
$parts = array_map(fn (string $part): string => trim($part, $this->escapeChar), explode('.', $item));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case also failed before the PR, but it appears to be a missing case within the intended fix. With an empty DBPrefix, the normalized $parts are not joined back into $item, so getFieldData('"public"."jobs"') still passes the quotes into the metadata query and returns no columns. Since Query Builder passes protected table names, could we cover this path too and add a test for a quoted, schema-qualified table name with an empty prefix?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants