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

Create DB 1.1 schema for system reference strings #564

Merged
merged 6 commits into from
Sep 15, 2020

Conversation

JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented Sep 11, 2020

Change

The primary motivation of this change was to add new fields for use in the list and upgrade commands; system reference strings that can be used to associate a winget package with software currently installed on a machine. This required creation of schema 1.1, as well as some refactoring to make inheritance and behavior changes easier between versions.

The two types of strings added are PackageFamilyName for MSIX packages, and ProductCode for packages that register through ARP (Add/Remove Programs). These are added with the option of having multiple of each in the event that a package's installers have unique values. The values are stored with their casing folded to allow for ordinal comparisons, a performance requirement due to future work on list and upgrade.

Additional noteworthy changes:

  1. Changed from implicit indices to explicit ones in 1.1, and drop more of them during preparation for packaging. This should result in a ~35% reduction in page count in the database, with no impact to performance.
  2. Added a dev debug tool in SQLiteWrapper.cpp; set WINGET_SQLITE_EXPLAIN_QUERY_PLAN_ENABLED to 1 to have the query plan for every SQLite prepared statement output to logging.

Validation

The existing tests have been updated to use GENERATE to run the tests not only against all versions, but also with backcompat scenarios.

Microsoft Reviewers: Open in CodeFlow

@JohnMcPMS JohnMcPMS marked this pull request as ready for review September 11, 2020 21:45
@JohnMcPMS JohnMcPMS requested a review from a team as a code owner September 11, 2020 21:45
// Interface to this schema version exposed through ISQLiteIndex.
struct Interface : public V1_0::Interface
{
// Version 1.0
Copy link
Contributor

@msftrubengu msftrubengu Sep 14, 2020

Choose a reason for hiding this comment

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

1.0 [](start = 19, length = 3)

1.1? #ByDesign

Copy link
Member Author

@JohnMcPMS JohnMcPMS Sep 14, 2020

Choose a reason for hiding this comment

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

No, this 1.0 refers to the interface version where the functions come from. These are 1.0 functions. If 1.1 added new accessor functions, they would be in a 1.1 section. #ByDesign

Copy link
Contributor

@msftrubengu msftrubengu left a comment

Choose a reason for hiding this comment

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

:shipit:

};
}

// The table for PackageFamilyName.
Copy link
Contributor

@yao-msft yao-msft Sep 14, 2020

Choose a reason for hiding this comment

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

PackageFamilyName [](start = 21, length = 17)

Nit: wrong description
#Resolved

auto foldIfNeeded = [](ApplicationMatchFilter& filter)
{
if ((filter.Field == ApplicationMatchField::PackageFamilyName || filter.Field == ApplicationMatchField::ProductCode) &&
filter.Type == MatchType::Exact)
Copy link
Contributor

@yao-msft yao-msft Sep 15, 2020

Choose a reason for hiding this comment

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

filter.Type == MatchType::Exact [](start = 16, length = 31)

Should we just fold regardless of Matchtype for these 2 fields? #ByDesign

Copy link
Member Author

Choose a reason for hiding this comment

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

The other MatchTypes use like for comparison, and so get folded as part of that. While it wouldn't hurt anything, it isn't necessary.


In reply to: 488313354 [](ancestors = 488313354)


if (!bindIndex)
{
AICLI_LOG(Repo, Verbose, << "ApplicationMatchField not supported in this version: " << ApplicationMatchFieldToString(field));
Copy link
Contributor

@yao-msft yao-msft Sep 15, 2020

Choose a reason for hiding this comment

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

"ApplicationMatchField not supported in this version: " [](start = 40, length = 55)

Do we want to surface this back to user in some form? #ByDesign

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is how a 1.0 database will avoid attempting to search for the new fields. It is an intended short circuit, but is logged for informational purposes.


In reply to: 488314712 [](ancestors = 488314712)

@@ -51,6 +51,10 @@ namespace AppInstaller::Repository::Microsoft
// Gets the schema version of the index.
Schema::Version GetVersion() const { return m_version; }

// Changes the version of the interface being used to operate on the database.
// Should only be used for testing.
void ForceVersion(const Schema::Version& version);
Copy link
Contributor

@yao-msft yao-msft Sep 15, 2020

Choose a reason for hiding this comment

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

void ForceVersion(const Schema::Version& version); [](start = 8, length = 50)

should we put it inside existing #def DISABLE***TEST_HOOK since it's test only? #Resolved

@@ -66,6 +66,12 @@ namespace AppInstaller::Manifest
// Store Product Id
string_t ProductId;

// Package family name for MSIX type only.
Copy link
Contributor

@yao-msft yao-msft Sep 15, 2020

Choose a reason for hiding this comment

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

MSIX [](start = 35, length = 5)

or Store type? #Resolved

Copy link
Contributor

@yao-msft yao-msft left a comment

Choose a reason for hiding this comment

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

:shipit:

@JohnMcPMS JohnMcPMS merged commit 412f144 into microsoft:master Sep 15, 2020
@JohnMcPMS JohnMcPMS deleted the trimindex branch September 15, 2020 21:14
@denelon
Copy link
Contributor

denelon commented Oct 1, 2021

Related to #1243

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.

4 participants