Skip to content

Skip unsupported column data types during schema discovery - #3802

Open
Joymax (joymaxnascimento) wants to merge 5 commits into
Azure:mainfrom
joymaxnascimento:fix/schema-discovery-honors-field-permissions
Open

Skip unsupported column data types during schema discovery#3802
Joymax (joymaxnascimento) wants to merge 5 commits into
Azure:mainfrom
joymaxnascimento:fix/schema-discovery-honors-field-permissions

Conversation

@joymaxnascimento

@joymaxnascimento Joymax (joymaxnascimento) commented Sep 4, 2026

Copy link
Copy Markdown

Why make this change?

  • Closes [Bug]: Table with a geometry column fails schema discovery ("GetFieldType returned null") even when fields.include excludes it #3801
    • A table holding a geometry column cannot be exposed at all. FillSchemaForTableAsync reads
      the object shape with SELECT *, and DbDataAdapter.FillSchema needs a CLR type for every
      column in the projection. Microsoft.Data.SqlClient resolves a CLR UDT's type through the
      Microsoft.SqlServer.Types assembly, which DAB does not reference, so the reader reports no
      type and the adapter fails with DataReader.GetFieldType(N) returned null. The entity never
      loads, and there is no configuration-side workaround — excluding the column through
      permissions does not help, because discovery runs before authorization is consulted.

What is this change?

Schema discovery leaves out columns whose data type the provider cannot map to a CLR type, so the
rest of the object stays reachable.

  • SqlMetadataProvider gains a virtual UnsupportedColumnDataTypes, empty by default. When it is
    empty the projection stays SELECT *, so PostgreSQL, MySQL and Cosmos are untouched.
  • MsSqlMetadataProvider overrides it with the three SQL Server CLR user-defined types:
    geometry, geography, hierarchyid.
  • When a provider declares such types, the column list is read from the existing GetColumnsAsync
    call — the Columns schema collection, catalog metadata only, so the offending type is never
    materialized — and the projection names the remaining columns. Identifiers come from the
    catalog's own COLUMN_NAME, so casing and quoting match the database rather than the config.
  • If nothing unsupported is present, or the column list cannot be obtained, the projection falls
    back to SELECT * and behavior is exactly what it is today.
  • Each skipped column is logged once per object at Warning level, naming the column and its type,
    so the omission is discoverable rather than silent.

Deliberately limited to three types

MsSqlQueryBuilder's autoentity discovery skips objects containing geography, geometry,
hierarchyid, sql_variant, xml, rowversion or vector. That list is intentionally not
reused here: SqlTypeConstants.SupportedSqlDbTypes marks timestamp and vector as supported,
and there are dedicated vector tests, so excluding them would regress shipped behavior. Only the
CLR UDTs actually make GetFieldType return null, and only those are skipped.

Why not honor fields.include instead

An earlier revision of this PR narrowed the projection to the columns the permissions allow to be
read. I withdrew it: read permission and physical existence are different things, and several
paths need a column to be in SourceDefinition.Columns without needing it to be readable.
A database policy on an excluded column stops parsing against the EDM model
(EdmModelBuilder iterates Columns) and returns 400 on every request in that role; entities
sharing a source.object share one SourceDefinition, so a per-entity filter cannot isolate
anything; multiple-create indexes Columns by relationship column name; REST PUT stops nulling
the hidden columns. Skipping by type has none of that blast radius, because a column the provider
cannot type was never usable in any of those paths.

Compatibility impact

Skipping is provider-wide and driven by the column's data type alone, so it does not consult
permissions: a geometry, geography or hierarchyid column is left out even for a role that
grants unrestricted read, and fields.include / fields.exclude no longer affect discovery in
any way.

That is a deliberate widening, and it takes nothing away in practice. Those three types cannot be
read today at all — the object fails discovery and the entity never loads, which is #3801. The
observable change is that such an entity now loads with the column absent, instead of not
loading. No column that previously reached SourceDefinition.Columns stops reaching it, so REST,
GraphQL, OpenAPI, MCP and database policies see exactly what they see today for every object that
works today. Providers other than MSSQL declare no unsupported types and keep SELECT *.

The one thing worth reviewing is discoverability: an omitted column is now a Warning in the log
rather than a startup failure. I chose the Warning deliberately, but if you would rather the
engine keep failing loudly and require an explicit opt-in per entity, that is a small change and
I am happy to make it.

How was this tested?

  • Integration Tests
  • Unit Tests

ValidateUnsupportedColumnTypeIsNotInferred in
src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs infers metadata against a live MSSQL
instance and asserts that the geometry column is absent from the inferred SourceDefinition
while the other columns are present. The entity places no field restriction, so the test covers
the column being skipped on the strength of its type alone. Before this change the same fixture
fails during InitializeAsync.

To be transparent: I have not run this test locally — it needs a provisioned SQL Server and I do
not have a disposable instance available. It compiles and reaches the fixture setup, but its
result has not been observed. Please let CI run it, and tell me if it needs adjusting.

The fixture follows the pattern already used for vector columns:

  • src/Service.Tests/DatabaseSchema-MsSql.sql — new geometry_type_table (drop, create, seed).
  • config-generators/mssql-commands.txtadd GeometryType.
  • src/Service.Tests/dab-config.MsSql.json — the regenerated entity.
  • src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt
    — refreshed, since the config gained an entity.

MSSQL only. No MySQL, PostgreSQL, DW SQL or Cosmos fixture changes, so those suites are untouched.

Sample Request(s)

Table:

create table dbo.Geom
(
    Id       int identity(1,1) not null primary key,
    Name     nvarchar(100)     not null,
    Location geometry          null
);

Entity — no special configuration needed:

"Geom": {
  "source": { "object": "dbo.Geom", "type": "table" },
  "permissions": [
    { "role": "anonymous", "actions": [ { "action": "read" } ] }
  ]
}

Before this change the engine fails while reading metadata for Geom. After it, the entity loads
and responds:

GET /api/Geom
{ "value": [ { "Id": 1, "Name": "Shaft collar" } ] }

with this in the log at startup:

warn: Skipping column(s) of dbo.Geom whose data type is not supported: Location (geometry).
      They are not exposed through REST, GraphQL or MCP.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@joymaxnascimento

Copy link
Copy Markdown
Author

Joymax (Joymax (@joymaxnascimento)) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

Contribution License Agreement

This Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”), and conveys certain license rights to Microsoft Corporation and its affiliates (“Microsoft”) for Your contributions to Microsoft open source projects. This Agreement is effective as of the latest signature date below.

  1. Definitions.
    “Code” means the computer software code, whether in human-readable or machine-executable form,
    that is delivered by You to Microsoft under this Agreement.
    “Project” means any of the projects owned or managed by Microsoft and offered under a license
    approved by the Open Source Initiative (www.opensource.org).
    “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any
    Project, including but not limited to communication on electronic mailing lists, source code control
    systems, and issue tracking systems that are managed by, or on behalf of, the Project for the purpose of
    discussing and improving that Project, but excluding communication that is conspicuously marked or
    otherwise designated in writing by You as “Not a Submission.”
    “Submission” means the Code and any other copyrightable material Submitted by You, including any
    associated comments and documentation.
  2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any
    Project. This Agreement covers any and all Submissions that You, now or in the future (except as
    described in Section 4 below), Submit to any Project.
  3. Originality of Work. You represent that each of Your Submissions is entirely Your original work.
    Should You wish to Submit materials that are not Your original work, You may Submit them separately
    to the Project if You (a) retain all copyright and license information that was in the materials as You
    received them, (b) in the description accompanying Your Submission, include the phrase “Submission
    containing materials of a third party:” followed by the names of the third party and any licenses or other
    restrictions of which You are aware, and (c) follow any other instructions in the Project’s written
    guidelines concerning Submissions.
  4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else
    for whom You are acting in making Your Submission, e.g. as a contractor, vendor, or agent. If Your
    Submission is made in the course of Your work for an employer or Your employer has intellectual
    property rights in Your Submission by contract or applicable law, You must secure permission from Your
    employer to make the Submission before signing this Agreement. In that case, the term “You” in this
    Agreement will refer to You and the employer collectively. If You change employers in the future and
    desire to Submit additional Submissions for the new employer, then You agree to sign a new Agreement
    and secure permission from the new employer before Submitting those Submissions.
  5. Licenses.
  • Copyright License. You grant Microsoft, and those who receive the Submission directly or
    indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license in the
    Submission to reproduce, prepare derivative works of, publicly display, publicly perform, and distribute
    the Submission and such derivative works, and to sublicense any or all of the foregoing rights to third
    parties.
  • Patent License. You grant Microsoft, and those who receive the Submission directly or
    indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license under
    Your patent claims that are necessarily infringed by the Submission or the combination of the
    Submission with the Project to which it was Submitted to make, have made, use, offer to sell, sell and
    import or otherwise dispose of the Submission alone or with the Project.
  • Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement.
    No additional licenses or rights whatsoever (including, without limitation, any implied licenses) are
    granted by implication, exhaustion, estoppel or otherwise.
  1. Representations and Warranties. You represent that You are legally entitled to grant the above
    licenses. You represent that each of Your Submissions is entirely Your original work (except as You may
    have disclosed under Section 3). You represent that You have secured permission from Your employer to
    make the Submission in cases where Your Submission is made in the course of Your work for Your
    employer or Your employer has intellectual property rights in Your Submission by contract or applicable
    law. If You are signing this Agreement on behalf of Your employer, You represent and warrant that You
    have the necessary authority to bind the listed employer to the obligations contained in this Agreement.
    You are not expected to provide support for Your Submission, unless You choose to do so. UNLESS
    REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING, AND EXCEPT FOR THE WARRANTIES
    EXPRESSLY STATED IN SECTIONS 3, 4, AND 6, THE SUBMISSION PROVIDED UNDER THIS AGREEMENT IS
    PROVIDED WITHOUT WARRANTY OF ANY KIND, INCLUDING, BUT NOT LIMITED TO, ANY WARRANTY OF
    NONINFRINGEMENT, MERCHANTABILITY, OR FITNESS FOR A PARTICULAR PURPOSE.
  2. Notice to Microsoft. You agree to notify Microsoft in writing of any facts or circumstances of which
    You later become aware that would make Your representations in this Agreement inaccurate in any
    respect.
  3. Information about Submissions. You agree that contributions to Projects and information about
    contributions may be maintained indefinitely and disclosed publicly, including Your name and other
    information that You submit with Your Submission.
  4. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and
    the parties consent to exclusive jurisdiction and venue in the federal courts sitting in King County,
    Washington, unless no federal subject matter jurisdiction exists, in which case the parties consent to
    exclusive jurisdiction and venue in the Superior Court of King County, Washington. The parties waive all
    defenses of lack of personal jurisdiction and forum non-conveniens.
  5. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and
    supersedes any and all prior agreements, understandings or communications, written or oral, between
    the parties relating to the subject matter hereof. This Agreement may be assigned by Microsoft.

@microsoft-github-policy-service agree

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Case-sensitive permission/primary-key filtering can incorrectly drop permitted columns or primary keys when config/schema casing differs, breaking metadata inference.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates SQL metadata discovery to honor entity field-level permissions when building the schema-read projection, avoiding provider failures on unsupported CLR-mapped columns (e.g., SQL Server geometry) that are not readable per config.

Changes:

  • Narrow FillSchemaForTableAsync from SELECT * to a permission-derived projection where possible, with fallbacks to preserve current behavior.
  • Re-apply per-entity column restrictions when populating SourceDefinition.Columns, accounting for the shared schema DataTable cache.
  • Add an MSSQL test fixture/table/config entity to validate that a geometry column omitted from fields.include is not inferred.
File summaries
File Description
src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Builds a permission-aware column projection for schema discovery and filters inferred columns accordingly.
src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs Adds an MSSQL test asserting excluded geometry column is not inferred.
src/Service.Tests/DatabaseSchema-MsSql.sql Adds geometry_type_table and seed data for the new test case.
src/Service.Tests/dab-config.MsSql.json Adds GeometryType entity with fields.include restricting readable columns.
src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt Updates snapshot to reflect the added MSSQL entity.
config-generators/mssql-commands.txt Adds generator command to create the GeometryType entity with fields.include.
Review details

Suppressed comments (1)

src/Core/Services/MetadataProviders/SqlMetadataProvider.cs:1904

  • Permitted column resolution uses case-sensitive sets (StringComparer.Ordinal) for Included/Excluded, but SourceDefinition.Columns is case-insensitive (StringComparer.InvariantCultureIgnoreCase). If config uses different casing than the schema (common with SQL Server), IsColumnPermitted() can incorrectly treat a readable column as non-permitted and drop it from SourceDefinition.Columns, breaking runtime behavior.
            HashSet<string> included = new(StringComparer.Ordinal);
            HashSet<string>? excluded = null;
            bool allColumns = false;
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Outdated
@joymaxnascimento

Copy link
Copy Markdown
Author

🟡 Changes recommended

Case-sensitive permission/primary-key filtering can incorrectly drop permitted columns or primary keys when config/schema casing differs, breaking metadata inference.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview
This PR updates SQL metadata discovery to honor entity field-level permissions when building the schema-read projection, avoiding provider failures on unsupported CLR-mapped columns (e.g., SQL Server geometry) that are not readable per config.

Changes:

  • Narrow FillSchemaForTableAsync from SELECT * to a permission-derived projection where possible, with fallbacks to preserve current behavior.
  • Re-apply per-entity column restrictions when populating SourceDefinition.Columns, accounting for the shared schema DataTable cache.
  • Add an MSSQL test fixture/table/config entity to validate that a geometry column omitted from fields.include is not inferred.

File summaries
File Description
src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Builds a permission-aware column projection for schema discovery and filters inferred columns accordingly.
src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs Adds an MSSQL test asserting excluded geometry column is not inferred.
src/Service.Tests/DatabaseSchema-MsSql.sql Adds geometry_type_table and seed data for the new test case.
src/Service.Tests/dab-config.MsSql.json Adds GeometryType entity with fields.include restricting readable columns.
src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt Updates snapshot to reflect the added MSSQL entity.
config-generators/mssql-commands.txt Adds generator command to create the GeometryType entity with fields.include.
Review details

Suppressed comments (1)

src/Core/Services/MetadataProviders/SqlMetadataProvider.cs:1904

  • Permitted column resolution uses case-sensitive sets (StringComparer.Ordinal) for Included/Excluded, but SourceDefinition.Columns is case-insensitive (StringComparer.InvariantCultureIgnoreCase). If config uses different casing than the schema (common with SQL Server), IsColumnPermitted() can incorrectly treat a readable column as non-permitted and drop it from SourceDefinition.Columns, breaking runtime behavior.
            HashSet<string> included = new(StringComparer.Ordinal);
            HashSet<string>? excluded = null;
            bool allColumns = false;
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Good catch. Fixed: Included/Excluded and the exposed-to-backing map now use
StringComparer.OrdinalIgnoreCase, matching the comparer on SourceDefinition.Columns and the
field name lookups in TryGetExposedColumnName/TryGetBackingColumn. The primary key guard
compares case-insensitively too, since SourceDefinition.PrimaryKey is a plain List.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new permission-based schema projection can omit primary key columns when PKs are inferred (not configured), causing initialization failures for valid configs that restrict fields.include without including PKs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The unconfigured-primary-key guard leaves the reported geometry scenario on SELECT *, while shared metadata and case-sensitive identifiers introduce additional correctness issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/Core/Services/MetadataProviders/SqlMetadataProvider.cs:2026

  • This guard disables the new behavior for the exact regression fixture: GeometryType has neither fields[].primary-key nor source.key-fields, so configuredPrimaryKey is empty, this returns unrestricted, and BuildSchemaProjectionAsync produces SELECT *. FillSchema will therefore still encounter geom and fail before the new test reaches its assertions. The primary key needs to be obtained without materializing every column (for example from catalog metadata) and unioned into the restricted projection; requiring it in configuration would not fix the reported configuration.
            if (entity.Permissions is null || entity.Permissions.Length == 0 || configuredPrimaryKey.Count == 0)
            {
                return new(AllColumns: true, included, new HashSet<string>(StringComparer.OrdinalIgnoreCase));
  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Outdated
Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Outdated
@joymaxnascimento

Copy link
Copy Markdown
Author

🟡 Changes recommended

The unconfigured-primary-key guard leaves the reported geometry scenario on SELECT *, while shared metadata and case-sensitive identifiers introduce additional correctness issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Thanks — all three points landed, and taken together they convinced me the approach was wrong
rather than incomplete. I have rewritten the change.

The PR now skips columns whose data type the provider cannot map to a CLR type, instead of
honoring field permissions. SqlMetadataProvider gains a virtual UnsupportedColumnDataTypes,
empty by default; MsSqlMetadataProvider overrides it with the three SQL Server CLR UDTs
(geometry, geography, hierarchyid). The column list comes from the existing GetColumnsAsync call,
so identifiers are the catalog's own and the offending type is never materialized. Everything
else falls back to SELECT *.

The reasoning: read permission and physical existence are different things. Several paths need a
column in SourceDefinition.Columns without needing it to be readable — a database policy parses
against the EDM model built from Columns, multiple-create indexes Columns by relationship column
name, REST PUT nulls the columns it finds there. Skipping by type has none of that blast radius,
because a column the provider cannot type was never usable in any of those paths anyway.

Your summary line was also right that the primary-key guard left the reported scenario on
SELECT *. That guard is gone; the new version fixes #3801 with no configuration required.

One note on scope: I did not reuse the type list from MsSqlQueryBuilder's autoentity discovery.
SqlTypeConstants.SupportedSqlDbTypes marks timestamp and vector as supported and there are
dedicated vector tests, so excluding them would regress shipped behavior. Only the CLR UDTs make
GetFieldType return null, and only those are skipped.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The implemented provider-wide type filtering materially differs from the documented permission-based behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs
@joymaxnascimento Joymax (joymaxnascimento) changed the title Honor field permissions when reading table schema Skip unsupported column data types during schema discovery Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Table with a geometry column fails schema discovery ("GetFieldType returned null") even when fields.include excludes it

2 participants