Skip to content

Conversation

@shueybubbles
Copy link
Collaborator

  • DesignMode fixes
  • Vector support

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces support for a new Vector data type and includes several DesignMode fixes. Key changes include:

  • Adding Vector handling across core code (SqlSmoObject, DataType, ParamBase, DataEnumerator) with associated default settings and type converters.
  • Updating QueryStoreOptions to support DesignMode and improving error handling.
  • Enhancing unit tests, test frameworks, and documentation to reflect the new support for Vector data types.

Reviewed Changes

Copilot reviewed 22 out of 28 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Microsoft/SqlServer/Management/Smo/SqlSmoObject.cs Added a new case for handling the Vector data type.
src/Microsoft/SqlServer/Management/Smo/QueryStoreOptions.cs Modified logic to handle DesignMode and adjusted conversion to use nameof for CapturePolicyExecutionCount.
src/Microsoft/SqlServer/Management/Smo/QueryStoreEnums.cs Reinstated type converters for the new QueryStore enums without functional changes.
src/Microsoft/SqlServer/Management/Smo/ParamBase.cs Extended SQL string conversion to include the Vector type.
src/Microsoft/SqlServer/Management/Smo/DataType.cs Updated the SqlDataType enum and constructors to add Vector with a default MaximumLength and proper SQL name mapping.
src/Microsoft/SqlServer/Management/Smo/DataEnumerator.cs Refactored column data handling and added formatting support for scripting Vector columns.
src/Microsoft/SqlServer/Management/Sdk/Sfc/Enumerator/Util.cs Included Vector in the mapping from DB type strings to CLR types.
src/FunctionalTest/Smo/ScriptingTests/Table_SmoTestSuite.cs Added tests to validate correct INSERT statement scripting for Vector columns.
src/FunctionalTest/Smo/ScriptingTests/SmoTestFramework/SmoTestBase.cs and Column_SmoTestSuite.cs Updated test configurations to support the new SQL Server version (v170) and Vector tests.
src/Codegen/README.md, docs/README.md, CHANGELOG.md Updated documentation and changelog to include information on DesignMode and Vector support.
Files not reviewed (6)
  • Packages.props: Language not supported
  • global.json: Language not supported
  • src/Codegen/SmoCodeGen.csproj: Language not supported
  • src/Codegen/cfg.xml: Language not supported
  • src/Directory.Build.props: Language not supported
  • src/Microsoft/SqlServer/Management/Smo/Microsoft.SqlServer.Smo.csproj: Language not supported
Comments suppressed due to low confidence (4)

src/Microsoft/SqlServer/Management/Smo/QueryStoreOptions.cs:242

  • Using newValue.Value directly may lead to a null reference if queryCaptureModeProp.Value is null. Consider adding a null-check or providing a default value before accessing .Value.
queryStoreCaptureModeTypeConverter.ConvertToInvariantString(newValue.Value)));

src/Microsoft/SqlServer/Management/Smo/DataType.cs:125

  • [nitpick] The default MaximumLength for Vector is hardcoded to 1998; ensure this value is appropriate for all expected scenarios or consider making it configurable.
this.MaximumLength = 1998;

src/Microsoft/SqlServer/Management/Smo/DataEnumerator.cs:963

  • The calculation for Vector dimensions may throw unexpected results if columnData.MaxLength is less than 8; verify that such a scenario is impossible or add validation to guard against it.
int dimensions = (columnData.MaxLength - 8) / 4;

src/FunctionalTest/Smo/ScriptingTests/Table_SmoTestSuite.cs:4082

  • [nitpick] The condition used to determine when to include a Vector column in tests may be ambiguous; please double-check that it correctly targets the intended server configurations.
if (database.Parent.ServerType != DatabaseEngineType.Standalone || database.Parent.VersionMajor > 17)

@shueybubbles
Copy link
Collaborator Author

Fixes #193

@shueybubbles shueybubbles merged commit 17b6e32 into main Apr 28, 2025
4 checks passed
@shueybubbles shueybubbles deleted the team/sync branch April 28, 2025 16:25
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.

3 participants