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

Fix issue where schema extraction is broken for remote services. Closes #1497 #1502

Merged
merged 5 commits into from
Mar 7, 2022

Conversation

binaek
Copy link
Contributor

@binaek binaek commented Feb 23, 2022

No description provided.

@binaek binaek added the bug Something isn't working label Feb 23, 2022
@binaek binaek self-assigned this Feb 23, 2022
db/db_local/local_db_client.go Show resolved Hide resolved
schemas := connectionSchemaMap.UniqueSchemas()
query := c.buildSchemasQuery(schemas)

acquireSessionResult := c.AcquireSession(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to acquire session here, but the base implementation just calls

connection, err := c.dbClient.Conn(ctx)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because dbClient is not a public property in the DbClient struct

db/db_local/local_db_client.go Show resolved Hide resolved
}
}

func (c *LocalDbClient) buildSchemasQuery(hintSchemas []string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

what are hintSchemas?

just call it schemas

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

function comment to explain why this is different from the base version

@@ -194,6 +196,8 @@ func (c *LocalDbClient) GetSchemaFromDB(ctx context.Context) (*schema.Metadata,
return metadata, nil
}

// update schemaMetadata to add in all other schemas which have the same schemas as those we have loaded
// NOTE: this mutates schemaMetadata
Copy link
Contributor

Choose a reason for hiding this comment

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

feel free to remove the extra spaces from my suggested comment

}
}

func (c *LocalDbClient) buildSchemasQuery(exemplarSchemas []string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

just schemas will be fine

@binaek binaek linked an issue Mar 7, 2022 that may be closed by this pull request
@kaidaguerre kaidaguerre merged commit feab5ad into main Mar 7, 2022
@kaidaguerre kaidaguerre deleted the schema_extraction_1497 branch March 7, 2022 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schema extraction is broken for remote databases
2 participants