Give query case sensitive treatment in query hash

[osule]

What type of PR is this?

• Refactor
• Feature
• Bug Fix
• New Query Runner (Data Source)
• New Alert Destination
• Other

Description

Generating the query hash from the query text with no lowercasing of the query text
allows case-sensitive parameter values in the dashboard to have different cache entries.

Related Tickets & Documents

Fixes #2137

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

[arikfr]

Thank you for opening this pull request. I've added some comments in case you are interested to follow up on it.

[guidopetri]

@osule , thanks for the PR! We've updated a lot of things now that we're Community-driven so - if you're still interested in getting this merged - would you mind rebasing off master to re-run the CI?

We're trying to clean up our PR todo list, so if you're not interested, that's fine - we'll close the PR in about a week if we don't hear back. If you're interested in reopening the PR afterwards, we would also very much welcome that.

[justinclift]

Looking over this PR, the requested changes were made so it should be ok to approve.

Looks like it's not passing our CI tests though, so some tweaking seems like it's still needed.

---

The failing CI seems to be a GitHub Actions infrastructure problem (yet again):

URL: https://download.cypress.io/desktop/5.3.0?platform=linux&arch=x64
Error: Failed downloading the Cypress binary.
Response code: 502
Response message: Bad Gateway

Re-running it should probably fix that. Hopefully the GitHub Actions infrastructure problem is only short lived today...

---

That failure of the frontend-lint tests was indeed just a temporary thing and hasn't happened again. This PR is showing a (single) failure in the backend-lint tests though:

=========================== short test summary info ============================
FAILED tests/test_migrations.py::test_only_single_head_revision_in_migrations
========== 1 failed, 735 passed, 65480 warnings in 246.71s (0:04:06) ===========

Looking at the details for that failure, it has:

_________________ test_only_single_head_revision_in_migrations _________________

self = <alembic.script.base.ScriptDirectory object at 0x7f0502fb13d0>
ancestor = None
multiple_heads = 'The script directory has multiple heads (due to branching).Please use get_heads(), or merge the branches using alembic merge.'
start = None, end = None, resolution = None

    @contextmanager
    def _catch_revision_errors(
        self,
        ancestor: Optional[str] = None,
        multiple_heads: Optional[str] = None,
        start: Optional[str] = None,
        end: Optional[str] = None,
        resolution: Optional[str] = None,
    ) -> Iterator[None]:
        try:
>           yield

/usr/local/lib/python3.8/site-packages/alembic/script/base.py:246: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/local/lib/python3.8/site-packages/alembic/script/base.py:395: in get_current_head
    return self.revision_map.get_current_head()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <alembic.script.revision.RevisionMap object at 0x7f0502fb1370>
branch_label = None

    def get_current_head(
        self, branch_label: Optional[str] = None
    ) -> Optional[str]:
        """Return the current head revision.
    
        If the script directory has multiple heads
        due to branching, an error is raised;
        :meth:`.ScriptDirectory.get_heads` should be
        preferred.
    
        :param branch_label: optional branch name which will limit the
         heads considered to those which include that branch_label.
    
        :return: a string revision number.
    
        .. seealso::
    
            :meth:`.ScriptDirectory.get_heads`
    
        """
        current_heads: Sequence[str] = self.heads
        if branch_label:
            current_heads = self.filter_for_lineage(
                current_heads, branch_label
            )
        if len(current_heads) > 1:
>           raise MultipleHeads(
                current_heads,
                "%s@head" % branch_label if branch_label else "head",
            )
E           alembic.script.revision.MultipleHeads: Multiple heads are present for given argument 'head'; 9aafdb8de771, fd4fc850d7ea

/usr/local/lib/python3.8/site-packages/alembic/script/revision.py:491: MultipleHeads

The above exception was the direct cause of the following exception:

    def test_only_single_head_revision_in_migrations():
        """
        If multiple developers are working on migrations and one of them is merged before the
        other you might end up with multiple heads (multiple revisions with the same down_revision).
    
        This makes sure that there is only a single head revision in the migrations directory.
    
        Adopted from [https://blog.jerrycodes.com/multiple-heads-in-alembic-migrations/.](https://blog.jerrycodes.com/multiple-heads-in-alembic-migrations/)
        """
        config = Config(os.path.join("migrations", "alembic.ini"))
        config.set_main_option("script_location", "migrations")
        script = ScriptDirectory.from_config(config)
    
        # This will raise if there are multiple heads
>       script.get_current_head()

tests/test_migrations.py:21: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/local/lib/python3.8/site-packages/alembic/script/base.py:395: in get_current_head
    return self.revision_map.get_current_head()
/usr/local/lib/python3.8/contextlib.py:131: in __exit__
    self.gen.throw(type, value, traceback)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <alembic.script.base.ScriptDirectory object at 0x7f0502fb13d0>
ancestor = None
multiple_heads = 'The script directory has multiple heads (due to branching).Please use get_heads(), or merge the branches using alembic merge.'
start = None, end = None, resolution = None

    @contextmanager
    def _catch_revision_errors(
        self,
        ancestor: Optional[str] = None,
        multiple_heads: Optional[str] = None,
        start: Optional[str] = None,
        end: Optional[str] = None,
        resolution: Optional[str] = None,
    ) -> Iterator[None]:
        try:
            yield
        except revision.RangeNotAncestorError as rna:
            if start is None:
                start = cast(Any, rna.lower)
            if end is None:
                end = cast(Any, rna.upper)
            if not ancestor:
                ancestor = (
                    "Requested range %(start)s:%(end)s does not refer to "
                    "ancestor/descendant revisions along the same branch"
                )
            ancestor = ancestor % {"start": start, "end": end}
            raise util.CommandError(ancestor) from rna
        except revision.MultipleHeads as mh:
            if not multiple_heads:
                multiple_heads = (
                    "Multiple head revisions are present for given "
                    "argument '%(head_arg)s'; please "
                    "specify a specific target revision, "
                    "'<branchname>@%(head_arg)s' to "
                    "narrow to a specific head, or 'heads' for all heads"
                )
            multiple_heads = multiple_heads % {
                "head_arg": end or mh.argument,
                "heads": util.format_as_comma(mh.heads),
            }
>           raise util.CommandError(multiple_heads) from mh
E           alembic.util.exc.CommandError: The script directory has multiple heads (due to branching).Please use get_heads(), or merge the branches using alembic merge.

/usr/local/lib/python3.8/site-packages/alembic/script/base.py:272: CommandError

I'm not really sure what to make of it. 😦

---

@osule If you're interested in taking a look at this, we recently re-created new instructions on how to set up a local development environment + test things:

https://github.com/getredash/redash/wiki/Local-development-setup

We know for sure those instructions works, as we use them pretty much daily. 😄

[osule]

@justinclift Thanks for trying to get this.
I'll take a look at the information you have shared. I'll regenerate the migration against the latest version.

[codecov]

Codecov Report

Merging #4254 (59e6099) into master (02d128e) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4254   +/-   ##
=======================================
  Coverage   59.71%   59.71%           
=======================================
  Files         151      151           
  Lines       12261    12261           
  Branches     1658     1658           
=======================================
  Hits         7322     7322           
  Misses       4733     4733           
  Partials      206      206           

Impacted Files	Coverage Δ
redash/utils/__init__.py	71.09% <100.00%> (ø)

[justinclift]

Thanks heaps for fixing that CI failure @osule. Merging this now. 😄