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

Implement $addFields stage dsl #103

Merged
merged 9 commits into from
Oct 29, 2023

Conversation

username1103
Copy link
Contributor

resolve #79


Currently, the $sum operator is implemented as follows.

But We need to use it as follows.

db.scores.aggregate( [
   {
     $addFields: {
       totalHomework: { $sum: "$homework" } ,
       totalQuiz: { $sum: "$quiz" }
     }
   },
   {
     $addFields: { totalScore:
       { $add: [ "$totalHomework", "$totalQuiz", "$extraCredit" ] } }
   }
] )

Comment on lines 21 to 23
/**
* Adds new fields to documents with aggregation expression.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add the @param docs for all methods.

* Adds new fields to documents from a field.
*/
infix fun String.setByField(fieldPath: String) {
builder.addField(this).withValue("$$fieldPath")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is withValueOf method for field path.
But test case is failed if used it.
Bad luck 😢

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. So use withValue 🥲

Comment on lines 16 to 21
data class Specs(val doors: Long?, val wheels: Long?, @Field("fuel_type") val fuelType: String?)
data class Vehicle(val id: Long, val type: String, val specs: Specs?)

data class Animal(val id: Long, val dogs: Long, val cats: Long)

data class Fruit(@Field("_id") val id: String, val item: String, val type: String)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use @Document annotation, then it will be more convenient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even when using the @Document annotation, it doesn't recognize 'id' as '_id'. I think it might be possible to achieve this by modifying the toFieldName according to the content of the following document.

https://docs.spring.io/spring-data/mongodb/docs/2.1.3.RELEASE/reference/html/#mongo-template.id-handling

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use @id annotation :) (same as remaining)

Copy link
Contributor Author

@username1103 username1103 Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even when using the @Id anntation, it doesn't recognize 'id' as '_id' in this case.

    fun overwriteAnExistingFieldWithField(): AggregationResults<Fruit> {
        val aggregation = aggregation {
            addFields {
                Fruit::id set Fruit::item
                Fruit::item set "fruit"
            }
        }

        return mongoTemplate.aggregate<Fruit>(aggregation, FRUITS)
    }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, then you can update toFieldName to read @id annotation as document.

Copy link
Contributor Author

@username1103 username1103 Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update toFieldName to check @Id annotation.

spring data monogo also has _id handling rule that a property or field without an annotation but named id maps to the _id field. docs

Shouldn't we add this rule?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and there are some hardcoded "_id" literal in example repositories.
you can update them too.

Copy link
Contributor

@jbl428 jbl428 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example that use $sum operator will be added when it is implemented completely.
I think that there might be some changes after this PR is merged.

LTGM 👍

@jbl428 jbl428 merged commit a5242da into inflearn:main Oct 29, 2023
1 check passed
@username1103 username1103 deleted the add-fields-stage-dsl branch October 29, 2023 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement $addFields stage dsl
2 participants