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

Invalid SQL generated in postgres using group with attributes #3009

Closed
tmont opened this Issue Jan 26, 2015 · 4 comments

Comments

4 participants
@tmont
Contributor

tmont commented Jan 26, 2015

This might be related (or the same as) #2968. Can't really tell if it's the same cause with a different effect.

Basically, I'm getting an error when using the postgres dialect and trying to do something like "count number of comments per post".

Here's my test code:

var Sequelize = require('sequelize'),
    extend = require('extend');

var sequelize = new Sequelize('db', 'username', 'password', {
    host: 'host',
    port: 5432,
    dialect: 'postgres',
    native: true,
    define: {
        charset: 'utf8'
    }
});

var options = {
    timestamps: false,
    freezeTableName: true,
    schema: 'schema',
    underscored: true
};

var Post = sequelize.define('Post', {
    id: { type: Sequelize.INTEGER, autoIncrement: true, primaryKey: true },
    name: { type: Sequelize.STRING, allowNull: false }
}, extend({}, options, { tableName: 'test_post' }));

var Comment = sequelize.define('Comment', {
    id: { type: Sequelize.INTEGER, autoIncrement: true, primaryKey: true }
}, extend({}, options, { tableName: 'test_comment' }));

Post.hasMany(Comment);

var query = Post.findAll({
    attributes: [ [ Sequelize.fn('COUNT', Sequelize.col('Comments.id')), 'comment_count' ] ],
    include: [
        { model: Comment, attributes: [] }
    ],
    group: [ 'Post.id' ]
});

query
    .then(function(rows) {
        console.log(require('util').inspect(rows, false, null, true));
    })
    .catch(function(err) {
        console.error(err);
    })
    .done(function() {
        process.exit();
    });

It's generating this SQL:

SELECT
  "Post"."id", 
  COUNT("Comments"."id") AS "comment_count", 
  "Comments"."id" AS "Comments.id" 
FROM "schema"."test_post" AS "Post" 
LEFT OUTER JOIN "schema"."test_comment" AS "Comments"
  ON "Post"."id" = "Comments"."post_id" 
GROUP BY "Post"."id";

The error postgres spits out is:

 [SequelizeDatabaseError: ERROR:  column "Comments.id" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: ...."id", COUNT("Comments"."id") AS "comment_count", "Comments"...
                                                             ^
]
  name: 'SequelizeDatabaseError',
  message: 'ERROR:  column "Comments.id" must appear in the GROUP BY clause or be used in an aggregate function\nLINE 1: ...."id", COUNT("Comments"."id") AS "comment_count", "Comments"...\n                                                             ^\n',
  parent:
   { [Error: ERROR:  column "Comments.id" must appear in the GROUP BY clause or be used in an aggregate function
   LINE 1: ...."id", COUNT("Comments"."id") AS "comment_count", "Comments"...
                                                                ^
   ]
     severity: 'ERROR',
     sqlState: '42803',
     messagePrimary: 'column "Comments.id" must appear in the GROUP BY clause or be used in an aggregate function',
     statementPosition: '64',
     sourceFile: 'parse_agg.c',
     sourceLine: '911',
     sourceFunction: 'check_ungrouped_columns_walker',
     sql: 'SELECT "Post"."id", COUNT("Comments"."id") AS "comment_count", "Comments"."id" AS "Comments.id" FROM "adstudio"."test_post" AS "Post" LEFT OUTER JOIN "adstudio"."test_comment" AS "Comments" ON "Post"."id" = "Comments"."post_id" GROUP BY "Post"."id";' },
  original:
   { [Error: ERROR:  column "Comments.id" must appear in the GROUP BY clause or be used in an aggregate function
   LINE 1: ...."id", COUNT("Comments"."id") AS "comment_count", "Comments"...
                                                                ^
   ]
     severity: 'ERROR',
     sqlState: '42803',
     messagePrimary: 'column "Comments.id" must appear in the GROUP BY clause or be used in an aggregate function',
     statementPosition: '64',
     sourceFile: 'parse_agg.c',
     sourceLine: '911',
     sourceFunction: 'check_ungrouped_columns_walker',
     sql: 'SELECT "Post"."id", COUNT("Comments"."id") AS "comment_count", "Comments"."id" AS "Comments.id" FROM "schema"."test_post" AS "Post" LEFT OUTER JOIN "schema"."test_comment" AS "Comments" ON "Post"."id" = "Comments"."post_id" GROUP BY "Post"."id";' },
  sql: 'SELECT "Post"."id", COUNT("Comments"."id") AS "comment_count", "Comments"."id" AS "Comments.id" FROM "schema"."test_post" AS "Post" LEFT OUTER JOIN "schema"."test_comment" AS "Comments" ON "Post"."id" = "Comments"."post_id" GROUP BY "Post"."id";'}

Basically the generated SQL has the Comments.id column in the SELECT statement even though it shouldn't be. Postgres can't handle that (I think MySQL just ignores it).

Anyway, I can't tell if this is a bug or if I'm doing something wrong. At the very least, sequelize probably shouldn't generate invalid SQL.

Here are the table definitions:

$ \d test_comment
 Column  |  Type   |                         Modifiers
---------+---------+-----------------------------------------------------------
 id      | integer | not null default nextval('test_comment_id_seq'::regclass)
 post_id | integer | not null
Indexes:
    "test_comment_pkey" PRIMARY KEY, btree (id)
Foreign-key constraints:
    "fk_test_comment_post" FOREIGN KEY (post_id) REFERENCES test_post(id) ON UPDATE CASCADE ON DELETE CASCADE

$ \d test_post
 Column |          Type          |                       Modifiers
--------+------------------------+--------------------------------------------------------
 id     | integer                | not null default nextval('test_post_id_seq'::regclass)
 name   | character varying(255) |
Indexes:
    "test_post_pkey" PRIMARY KEY, btree (id)
Referenced by:
    TABLE "test_comment" CONSTRAINT "fk_test_comment_post" FOREIGN KEY (post_id) REFERENCES test_post(id) ON UPDATE CASCADE ON DELETE CASCADE

I generated this using sequelize@2.0.0-rc8.

@davidrapin

This comment has been minimized.

Contributor

davidrapin commented Oct 14, 2015

Experiencing the same bug here. Any news on this @mickhansen?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Oct 14, 2015

@davidrapin Aggregates are hard to express in an ORM but can be done if you limit the attributes to what is absolutely needed and have the correct group statements, what is your issue specifically @davidrapin?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Oct 14, 2015

This specific issue should actually be used since include.attributes = [] should result in no selects at all these days.

@eeun

This comment has been minimized.

eeun commented Oct 31, 2015

I was having the same problem. It looks like there are 3 places where the primary key is added to the attributes list (model.js: 543, 638, 1364). I'd guess there should be a check for aggregate functions in the columns list before adding the primary key. In the meantime I solved my issue by adding "raw:true" to the query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment