sequelize / sequelize-typescript

Decorators and some other features for sequelize
MIT License
2.79k stars 282 forks source link

Group should be included in include if seperate:true is specified #613

Closed chaitanya11 closed 5 years ago

chaitanya11 commented 5 years ago

If seperate: true is specified, user should be able to do group operation, because it will be executed as separate query. related issue in Sequlize.

stack overflow question

RobinBuschmann commented 5 years ago

Hey @chaitanya11, thanks for bringing this up. Can you fill out the issue template please? I don't quite understand your issue. Are you expecting this to work with sequelize-typescript while it doesn't?

chaitanya11 commented 5 years ago
QUESTION_MAIN.findAll({
attributes:['QUESTION_ID', 'POSTER_I',
  ['( 6371 * acos( ' 
                  + 'cos( radians('+qstnFeedRequest.qstnLocLat+') ) ' 
                  + '* cos( radians( QSTN_LOC_LAT ) ) '
                  + '* cos( radians( QSTN_LOC_LONG ) - radians('+ qstnFeedRequest.qstnLocLong+') ) '
                  + '+ sin( radians('+qstnFeedRequest.qstnLocLat+') ) '
                  + '* sin( radians( QSTN_LOC_LAT ) ) ) '
  + ')', 'DISTANCE'
  ]
],
include: [
  { model: USER_ATTRIB, 
    attributes:['USER_NAME']
  },
  { model: QSTN_CATG, 
    attributes: [['CATG_I', 'QSTN_CATG_I']],
    where: qstnCatgWhereClause
  },
  { model: REPLY_MAIN, 
    attributes: ['QSTN_I', [sequelize.fn('COUNT', sequelize.col('REPLY_MAIN.QSTN_I')),   'REPLY_COUNT']], 
    where: {REPLY_STATUS: 200},
       group: ['QSTN_I'], // Should be enabled if it is a separate query
       separate: true, // separate query
    having: {'REPLY_COUNT': {$ne: null}}, 
    required: false
  }
],
having:{ 'DISTANCE' : {$lt: 5} },
where: whereClause,
limit: qstnFeedRequest.limit
})

in above mentioned operation we are including REPLY_MAIN Model in a separate query. so shoulbe be executed in a separate query like follows SELECT * FROM REPLY_MAIN WHERE REPLY_STATUS=200 GROUP BY QSTN_I HAVING REPLY_COUNT NOT NULL

But, in sequelize-typescript even after enabling separate: true, Group by and Having were not enabled giving compilation error in code.

chaitanya11 commented 5 years ago

@RobinBuschmann let us know your thoughts about this.

RobinBuschmann commented 5 years ago

@chaitanya11 Which sequelize and which sequelize-typescript are you using? Did you try the same with pure sequelize - does it work there?

chaitanya11 commented 5 years ago

@RobinBuschmann here is the sequelize version "sequelize": "^5.8.6" working code with sequelize


const Sequelize = require('sequelize');

var sequelize = new Sequelize('database', 'username', 'pasword', {
    host: 'localhost',
    dialect: 'mysql',
});

var Person = sequelize.define('Person', {

    name: Sequelize.STRING
});

var Father = sequelize.define('Father', {

    age: Sequelize.STRING,
});

Person.hasMany(Father);

const queyFn = async () => {
    const result = await Person.findAll({
        include: [
            {
                model: Father,
                attributes: {
                    include: [[sequelize.fn('COUNT', sequelize.col('age')),   'age_count']]
                }, 
                separate: true,
                group: ['id']
            }
        ]
    });
    console.log(result.dataValues);
};

queyFn();

the above program translate query to

Executing (default): SELECT `Person`.`id`, `Person`.`name`, `Person`.`createdAt`, `Person`.`updatedAt` FROM `People` AS `Person`;
Executing (default): SELECT `id`, COUNT(`age`) AS `age_count`, `PersonId` FROM `Fathers` AS `Father` WHERE `Father`.`PersonId` IN (1, 2, 3, 4, 5) GROUP BY `id`;

while "sequelize-typescript": "^0.6.8", will give compilation error Object literal may only specify known properties, and 'group' does not exist in type 'typeof Model | IIncludeOptions'.

Unable to add group: ['id'] in include when separate: true.

Adding sequelize project zip for reference : tmp.zip

RobinBuschmann commented 5 years ago

@chaitanya11 Thanks. It's obviously working, but unfortunately I can't find anything in the docs of sequelize. Can you provide a link to it? Nevertheless I mark this as a bug - this should be added to the typings of sequelize-typescript. But also to the typings of sequelize@5, there are missing there as well.

chaitanya11 commented 5 years ago

@RobinBuschmann so should we raise this bug in sequelize@5 and wait till they resolve and release new version or shall we try to solve in our sequelize-typescript repo and make that as reference to sequelize buddies ?

RobinBuschmann commented 5 years ago

@chaitanya11 I think both makes sense: Fixing it in sequelize-typescript (what you already did) and raising a bug in sequelize.