search

ankane / blazer

Business intelligence made simple
MIT License
4.56k stars 474 forks source link

Stop establishing the connection as part of `preview_statement` #351

Closed bnjix closed 2 years ago

bnjix commented 3 years ago

Problem

#preview_statement is called in the queries/_form partial.

That has the side effect of needing to establish a connection for all sources that use the SqlAdapter as part of the sqlserver? method.

This causes the queries/new and queries/edit to become slow when one has a lot of data sources.

Solution

Use the url to determine if the connection is SQL Server or not

ankane commented 3 years ago

Hey @bnjix, thanks for the PR. However, it looks like the change doesn't cover all adapters. The connection should already be established on most page loads, so I'm not sure it'll make a big difference.

bnjix commented 2 years ago

Hey @ankane thanks for the reply here!

It is true that this change does not cover all adapters. But in a case where one has most/all of its data sources being SQL this improves the load times significantly.

The connection should already be established on most page loads

For queries/new and queries/edit, the only reason the connections are required is to know how to populate the preview statements for each DB, and that's why this change - by removing the need for the connection - greatly improves the performance of these 2 pages if one has many data sources.

ankane commented 2 years ago

Hey @bnjix, thanks for the follow up. By not covering all adapters, I meant it replaces a check for 3 different SQL Server adapters (SQLServer, tinytds, mssql), with one (jdbc:sqlserver://), which will break existing installations.