Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.0.1] RevEng: Ignore column store index #23421

Merged
merged 1 commit into from
Nov 24, 2020
Merged

Conversation

smitpatel
Copy link
Member

@smitpatel smitpatel commented Nov 20, 2020

Since all the columns are considered included columns

Resolves #23378

Description

All the columns in the columnstore index in SqlServer appears as included columns for the index. When we started identifying included columns as separate and started skipping them over, the columnstore indexes were left without any columns in the index causing error when generating model out of it.

Customer Impact

Customer will see an exception when scaffolding a database which has a columnstore index without any details why it failed. It has only been reported by a single customer so far, but we believe more people will hit this given that it also reproduces when scaffolding from the WideWorldImporters sample database, which is commonly used in SQL Server samples.

How found

Customer reported on 5.0.

Test coverage

Added test to skip column store index. As I mentioned in another issue, we have identified scaffolding (a.k.a. reverse engineering) as an area where we are lacking test coverage and are beginning to address this. One of the things we have already done is to reverse engineer four standard sample databases (Northwind, Pubs, AdventureWorks, and WideWorldImporters) which together cover many SQL Server features. This issue is reproduced on WideWorldImporters; we have not found any further issues from these databases.

Regression?

Yes, in 3.1 these indexes were scaffolded as regular index, which was wrong but not harmful. In 5.0 RTM it throws exception. With this fix they will be ignored. Ignoring them will allow us to add support for them fully in future.

Risk

Low. We would skip an index only if it does not have any columns. And indexes without any columns will throw later anyway. Also added quirk to go back to previous behavior.

@ajcvickers
Copy link
Member

Approved by Tactics for 5.0.1.

@ajcvickers ajcvickers changed the title RevEng: Ignore column store index [5.0.1] RevEng: Ignore column store index Nov 22, 2020
Since all the columns are considered included columns

Resolves #23378
@smitpatel smitpatel changed the base branch from main to release/5.0 November 24, 2020 00:05
@smitpatel
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@smitpatel
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@smitpatel smitpatel merged commit 6ec33b4 into release/5.0 Nov 24, 2020
@smitpatel smitpatel deleted the smit/columnstoreindex branch November 24, 2020 16:02
@ajcvickers ajcvickers removed this from the 5.0.1 milestone Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.ArgumentException: The collection argument 'propertyNames' must contain at least one element.
3 participants