Skip to content

fix: fresh headers per paged request, cap SQL results at TOP (#172) - #175

Open
zekelinAlex wants to merge 1 commit into
TALXIS:masterfrom
zekelinAlex:users/alexander.zekelin/fix-sql-query-cookie-issue-172
Open

zekelinAlex wants to merge 1 commit into
TALXIS:masterfrom
zekelinAlex:users/alexander.zekelin/fix-sql-query-cookie-issue-172

Conversation

@zekelinAlex

@zekelinAlex zekelinAlex commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

ServiceClient.ExecuteWebRequest mutates the header dictionary it receives by adding a Cookie entry. We passed the same dictionary to every paged request, so fetching any @odata.nextLink page failed with "An item with the same key has already been added. Key: Cookie". Each request now gets a fresh dictionary, in DataverseQueryService and in SolutionComponentQueryReader.

The ?sql= endpoint turns TOP n into a page size, not a row limit, so SELECT TOP 5 on a bigger table returned five rows plus a nextLink and hit the crash above. Following the links would also download the whole table in pages of five, so the client now reads TOP from the query and stops at that many rows; with --top, the smaller limit wins.

The FROM parser also accepts dbo.account and bracketed names like [account] now. The endpoint always took these, we just failed to extract the table name from them.

Close #172

@zekelinAlex zekelinAlex changed the title fix: fresh headers per paged request, cap SQL results at TOP fix: fresh headers per paged request, cap SQL results at TOP (#172) Jul 21, 2026

Copy link
Copy Markdown
Member

Implementation notes for the agent (TALXIS lab runbook, lane U4)

Why the lab needs it: this is lane U4 in TALXIS/lab#65; it ships with G3 but doesn't gate it. No lab script calls data query sql. Agents do, though, when they check their work in Dev, for example Product rows after CP11/CP12 or lastsyncedon after a rescan (#80). A Haiku-class agent's first try is txc env data query sql "SELECT TOP 5 … FROM <table>", and today any table larger than the page size crashes with "An item with the same key has already been added. Key: Cookie" (#172). solution component list paging (SolutionComponentQueryReader) has the same bug.

State: 1 commit, 13 behind master (base c2167a3). It merges cleanly onto master and with #201 (which also touches DataverseQueryService.cs). master + #179 + #139 + #175 + #169 builds, and all 752 unit tests pass (Oct 24). Closes #172. No tests were added.

Left to do

  1. Rebase onto master.
  2. Tests. TALXIS.CLI.Platform.Dataverse.Data already has InternalsVisibleTo TALXIS.CLI.Tests, so make TryGetSqlTop, MinTop and the FROM regex internal static and test them:
    • TOP 5, TOP (5), top 5;
    • no TOP;
    • MinTop(10, 5) == 5 and MinTop(null, 5) == 5;
    • FROM account, FROM dbo.account, FROM [dbo].[account], FROM [account];
    • a column named laptop doesn't count as a TOP clause.
  3. Headers: add a test or a code comment proving each ExecuteWebRequest call gets a new dictionary. A helper that builds headers per call is enough, as the PR does.
  4. Check the other callers that pass a header dictionary: SolutionDetailReader.cs:88, DataverseSolutionLayerQueryService.cs:55 and DataverseMetadataIdResolver.cs:18,45. They're single requests today, so they're safe. Leave a one-line comment, or use the same per-call helper, so nobody reuses one in a loop later.

Traps

  • TryGetSqlTop takes the first TOP n in the string. A subquery with its own TOP would cap the outer result. Document that limit in the --top help, or match only ^\s*SELECT\s+(DISTINCT\s+)?TOP. Prefer the anchored regex.
  • Keep the early break before reading the next page. Otherwise TOP 5 still downloads the whole table in pages of 5.

Order: independent. Merge before #201 is rebased (#201 is a later release).

Done when

  • dotnet test tests/TALXIS.CLI.Tests passes with the new tests.
  • Live: txc env data query sql "SELECT TOP 5 name FROM account" --profile dev --format json returns exactly 5 rows on a table with more than 5 rows, and no Cookie error. "SELECT name FROM dbo.account" works too. If you have no tenant, say so in the PR; Tom runs it.
  • The lab dry run against a locally packed txc passes (recipe: fix: solution import no longer hides build errors or imports stale zips #179 (comment)).

Generated by Claude Code

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: querying data through SQL doesn't work

2 participants