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

#with cannot be used with strings in 6.1 #42561

Open
dark-panda opened this issue Jun 22, 2021 · 6 comments
Open

#with cannot be used with strings in 6.1 #42561

dark-panda opened this issue Jun 22, 2021 · 6 comments

Comments

@dark-panda
Copy link
Contributor

Steps to reproduce

Using #with with strings instead of relations stopped working between Rails 6.0 and 6.1. Specifically, the following commit caused the break:

596469d

In Rails 6.0, you could do things like

popular_posts_from_cte = Post.with("popular_posts AS (SELECT * FROM posts WHERE views_count > 100)").from("popular_posts AS posts")

However, that broke in Rails 6.1 with the following exception as there is no case to handle strings in the case statement:

TypeError: no implicit conversion of nil into String

A patch to fix the issue is forth coming.

Expected behavior

The SQL should generate when strings are used in #with values.

Actual behavior

A TypeError is raised.

System configuration

Rails version: 6.1.3.2

Ruby version: 2.7.3

@HParker
Copy link
Contributor

HParker commented Jun 22, 2021

A patch to fix the issue is forth coming.

Thanks for taking on this patch @dark-panda! I look forward to reading it ❤️

I am a bit confused though, I didn't know there was a with method on model classes.

I don't seem to have it in a new rails app.

Article.method(:with)
NameError: undefined method `with' for class `#<Class:Article(id: integer, title: string, created_at: datetime, updated_at: datetime, user_id: integer)>'
from (pry):6:in `method'

Where is that method coming from? Is it another gem, or maybe I am doing something silly.

@ghiculescu
Copy link
Member

@eileencodes
Copy link
Member

Technically arel is a private API, I don't know if we want to allow strings in the with API.

@HParker
Copy link
Contributor

HParker commented Jun 22, 2021

Ah, so without activerecord-cte I have to do Article.all.arel.with("select 1"). activerecord-cte Seems to use a lot of arel. I wonder if activerecord-cte could be patched to handle strings instead of making a change in rails 🤔 .

@rafaelfranca
Copy link
Member

In this case I think it is fine since Arel used to work in this way and a new feature to arel we introduced broke it.

@dark-panda
Copy link
Contributor Author

dark-panda commented Jun 23, 2021

This was discovered while I was writing a patch for activerecord-cte to fix a kinda-sorta-related issue with the gem. That PR is up at vlado/activerecord-cte#3. Basically, all specs were working when testing against Rails 5.2 and 6.0 but then one particular test was failing in 6.1.

dark-panda added a commit to dark-panda/rails that referenced this issue Jun 27, 2021
dark-panda added a commit to dark-panda/rails that referenced this issue Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants