Skip to content

Fix MySQL aggregation issues: SET SESSION conflicts and pivot ordering#340

Closed
jas88 wants to merge 36 commits into
HicServices:mainfrom
jas88:fix/mysql-set-session-conflicts
Closed

Fix MySQL aggregation issues: SET SESSION conflicts and pivot ordering#340
jas88 wants to merge 36 commits into
HicServices:mainfrom
jas88:fix/mysql-set-session-conflicts

Conversation

@jas88

@jas88 jas88 commented Oct 20, 2025

Copy link
Copy Markdown
Contributor

Summary

Fixes multiple MySQL aggregation issues related to CTE scope, SET SESSION conflicts, and pivot column ordering.

Changes

  1. Fix CTE scope in dynamic SQL - Moved dateAxis CTE definition inside the CONCAT'd dynamic SQL to ensure it's in scope when referenced in PREPARE/EXECUTE statements

  2. Fix pivot column ordering - Added ROW_NUMBER() to preserve aggregate count-based ordering in GROUP_CONCAT operations, ensuring pivot columns appear by count descending instead of alphabetically

  3. Fix MySQL calendar aggregation - Combined CTEs correctly to avoid conflicts between SET SESSION and CTE declarations

  4. Resolve NUnit analyzer warnings - Fixed test code quality issues

Test Results

  • Test_Calendar_WithPivot(MySql,False) now passes
  • ✅ All MySQL aggregation tests passing

Test plan

  • MySQL pivot aggregation tests pass
  • Calendar aggregation tests pass
  • No regressions in other database implementations
  • Build succeeds with no errors

🤖 Generated with Claude Code

jas88 and others added 30 commits July 23, 2025 13:45
* Fix Oracle driver exception inconsistency in TestKeywords_Invalid

Oracle's ManagedDataAccess driver throws OracleException instead of
ArgumentException when encountering invalid connection string keywords,
while other database drivers throw ArgumentException.

Updated the test to accept either exception type using Assert.Catch<Exception>
and Assert.Multiple with an Or constraint, making the test resilient to
driver-specific exception behavior.

This resolves CI test failure:
- Expected: ArgumentException
- Actual: OracleException (ORA-50008: 'FLIBBLE' is an invalid connection string attribute)

The test now verifies:
1. Either ArgumentException OR OracleException is thrown
2. The exception message contains the invalid keyword "FLIBBLE"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix resource namespace mismatch for embedded resources

Backport fix from PR #3: The auto-generated Designer files were looking
for resources with namespace "FAnsi.*" but the actual embedded resources
used namespace "FAnsi.Core.*", causing MissingManifestResourceException.

Fixed by updating ResourceManager initialization in both:
- FAnsiStrings.Designer.cs: "FAnsi.FAnsiStrings" → "FAnsi.Core.FAnsiStrings"
- SR.Designer.cs: "FAnsi.SR" → "FAnsi.Core.SR"

This resolves the OneTimeSetUp error:
"Could not find the resource 'FAnsi.FAnsiStrings.resources' among the
resources 'FAnsi.Core.FAnsiStrings.resources'"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
This fork is not a HIC project. Removed "HIC." prefix.

Other build error/warning fixes.
Bumps [dotnet-sdk](https://github.com/dotnet/sdk) from 9.0.200 to 9.0.306.
- [Release notes](https://github.com/dotnet/sdk/releases)
- [Commits](dotnet/sdk@v9.0.200...v9.0.306)

---
updated-dependencies:
- dependency-name: dotnet-sdk
  dependency-version: 9.0.306
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4...v5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: Microsoft.Data.SqlClient
  dependency-version: 6.1.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: Microsoft.Data.SqlClient
  dependency-version: 6.1.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: Npgsql
  dependency-version: 9.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Npgsql
  dependency-version: 9.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: NUnit
  dependency-version: 4.4.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: NUnit3TestAdapter
  dependency-version: 5.2.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: Oracle.ManagedDataAccess.Core
  dependency-version: 23.26.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: Oracle.ManagedDataAccess.Core
  dependency-version: 23.26.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: System.Linq.Async
  dependency-version: 6.0.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: System.Linq.Async
  dependency-version: 6.0.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3 to 4.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@v3...v4)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-version: '4'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: NUnit.Analyzers
  dependency-version: 4.10.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/setup-dotnet](https://github.com/actions/setup-dotnet) from 4 to 5.
- [Release notes](https://github.com/actions/setup-dotnet/releases)
- [Commits](actions/setup-dotnet@v4...v5)

---
updated-dependencies:
- dependency-name: actions/setup-dotnet
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: System.Linq.Async
  dependency-version: 6.0.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: Npgsql
  dependency-version: 9.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
---
updated-dependencies:
- dependency-name: Microsoft.Data.SqlClient
  dependency-version: 6.1.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Addresses 8 CodeQL alerts for missing documentation summaries by adding
<summary> tags to methods that were using only <include> tags.

CodeQL requires explicit <summary> tags and doesn't recognize <include>
tags as documentation. This change adds brief summaries while preserving
the existing <include> tags for detailed documentation.

Files modified:
- FAnsi.Core/Discovery/IDiscoveredServerHelper.cs
- FAnsi.Core/Discovery/DiscoveredServerHelper.cs
- FAnsi.Core/Discovery/DiscoveredServer.cs
- FAnsi.Core/Discovery/DiscoveredTable.cs
- FAnsi.Core/Discovery/IDiscoveredTableHelper.cs

Resolves CodeQL alerts: #33-40

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Combined nested if statements in FAnsiExpressionVisitor.cs for cleaner logic flow
- Replaced if-else block with ternary operator in FAnsiQueryProvider.cs for more concise code

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
updated-dependencies:
- dependency-name: Oracle.ManagedDataAccess.Core
  dependency-version: 23.26.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
- MySQL: Replace legacy cross-join date generation with MySQL 8.0+ recursive CTEs
- MySQL: Optimize pivot implementation by combining GROUP_CONCAT calls
- MySQL: Remove 107 lines of dead commented code
- PostgreSQL: Implement BuildPivotOnlyAggregate and BuildPivotAndAxisAggregate using FILTER clause
- Oracle: Implement BuildPivotOnlyAggregate and BuildPivotAndAxisAggregate using CASE statements

This reduces MySQL complexity from 440 to 170 lines (61% reduction) and brings PostgreSQL and Oracle to feature parity with SQL Server for pivot operations.
- Add patterns for .claude/, .claude.json, CLAUDE.md and variants
- Add recursive patterns to catch Claude files in any subdirectory
- Remove .claude/ from git tracking if present
Combine multiple SET SESSION statements into a single statement
at the top of queries that use both recursive CTEs and pivot.
Prevents 'You have an error in your SQL syntax' errors when
both cte_max_recursion_depth and group_concat_max_len need to be set.

Fixes Test_Calendar_WithPivot(MySql,True) failure.
- Set cte_max_recursion_depth to 50000 (up from default 1000)
- Fix date generation termination to use <= instead of <
- Combine SET SESSION statements to avoid MySQL syntax errors
- Add skipSessionSettings parameter to avoid duplicate SET statements

Fixes Test_Calendar_Day, Test_Calendar_WithPivot and year overflow issues.
Replace implicit assumptions with explicit precondition checks that throw
InvalidOperationException when required properties are null. This provides
better error messages and fixes all nullable reference warnings in the file.
jas88 added 6 commits October 20, 2025 12:42
CTEs must be in the same statement as the SELECT that uses them.
Moved SET SESSION before the CTE+SELECT statement and removed
skipSessionSettings parameter since it's no longer needed.
- Make base test classes abstract (NUnit1034)
  - DatabaseTests
  - AggregationTests
- Fix Assert.That argument order (NUnit2007)
  - Swap actual and expected values in DatatypeComputerTests
  - Actual value should be first, expected value second
- Fix SQL format string bug in BuildAxisAggregate causing syntax errors
  - Changed {4} AS "{4}" to dataset.{3} AS "{3}" to properly select count column
  - Fixes Test_Calendar_Day and Test_Calendar_Quarter failures
- Add null-forgiving operators to MySqlBulkCopy to fix CS8602/CS8604 warnings
- Suppress IL3050 AOT warnings in query providers with UnconditionalSuppressMessage
  - IQueryProvider inherently requires dynamic code for generic type creation
MySQL doesn't allow multiple separate WITH statements. The dateAxis
and pivotValues CTEs must be combined into a single WITH clause with
comma-separated CTE definitions.

Fixes Test_Calendar_WithPivot failure.
CTEs only exist within a single SQL statement. Moving dateAxis
CTE definition into the CONCAT'd dynamic SQL ensures it's in scope
when the SELECT references it in the PREPARE/EXECUTE statement.

Fixes: Table 'FAnsiTests.dateAxis' doesn't exist error
Pivot columns were appearing alphabetically instead of by aggregate
count descending. Added ROW_NUMBER() to preserve the count-based
ordering in GROUP_CONCAT operations.

Fixes: Test_Calendar_WithPivot(MySql,False) expecting T before E
@jas88

jas88 commented Oct 20, 2025

Copy link
Copy Markdown
Contributor Author

Wrong repo - closing immediately

@jas88 jas88 closed this Oct 20, 2025
@jas88 jas88 deleted the fix/mysql-set-session-conflicts branch October 20, 2025 21:33
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.

1 participant