JPA/hibernate is generating the wrong SQL statement

Question

My JpaRepository is generating the wrong SQL. It only updates shop and not the rest of the entity. After looking closer I noticed the query say update shop set nothing about campaign entity in which shop is inside.

Inside Campaign there is a shop:

 @JoinColumn(name = "SHOP_ID", referencedColumnName = "SHOP_ID", nullable = false)
    @ManyToOne(cascade = CascadeType.ALL)
    private Shop shop;

and inside shop I have a set:

@OneToMany(cascade = CascadeType.ALL, mappedBy = "shop")
private Set<Campaign> campaignSet;

The query generated with a saveAndFlush -

update shop set created_at=?, currency=?, currency_iso_code=?, default_delivery_cost=?, delivery_cost_erp_number=?, description=?, default_language_id=?, modified_at=?, sales_organisation_id=?, code=?, valid_from=?, valid_till=? where shop_id=?

I want the whole Campaign to be saved and I honestly do not care about updating the shop because I will never do that when updating a campaign.

Method updating:

@Override
@Modifying
@Transactional
public CampaignDto update(CampaignDto campaignDto) throws RequestNotFoundException {
    //TODO: compare objects to see if there is a change

    Campaign campaign = mapper.mapReverse(campaignDto);

    if (campaign.getCampaignId() != null) {
        campaign = campaignRepository.getOne(campaign.getCampaignId());

        if (campaign.getCampaignId() == null) {
            throw new RequestNotFoundException(
                    String.format("Campaign %s is not found", campaign.getKey().toString()));
        }
    }

    Shop shop = shopRepository.getOne(campaignDto.getShopId());

    if (shop.getShopId() > 0 && shop.getShopId() != null) {
        shop.setCode(campaignDto.getShopCode());
        shop.setCurrency(campaignDto.getShopCurrency());
        shop.setCurrencyIsoCode(campaignDto.getShopCurrencyIso());
        shop.setValidFrom(campaignDto.getValidFrom());
        shop.setValidTill(campaignDto.getValidTill());
        shop.setCreatedAt(OffsetDateTime.now());
        shop.setSalesOrganisationId(campaignDto.getSalesOrganisationId());
    } else {
        throw new IllegalArgumentException("Must declare an existing shop to update a campaign.");
    }

    Language language = languageRepository.getOne(campaignDto.getLanguageId());
    if (!StringUtils.isEmpty(language.getLanguageId()) && language.getLanguageId() != null) {
        language.setLanguageId(campaignDto.getLanguageId());
        shop.setLanguage(language);
    } else {
        throw new IllegalArgumentException("Must declare an existing language to update a campaign.");
    }

    campaign.setShop(shop);
    CampaignDto updatedCampaign = mapper.map(campaign);
    campaignRepository.saveAndFlush(campaign);

    return updatedCampaign;
}

crizzis · Accepted Answer · 2018-04-30T12:44:37.213

I honestly do not care about updating the shop because I will never do that when updating a campaign

Why the CascadeType.ALL, then? You should never, ever use CascadeType.ALL with many-to-one associations. This is because CascadeType.MERGE rarely makes sense (you're essentially begging for unintended side effects like the one you describe), and CascadeType.REMOVE is just pure evil (by removing the child, you remove the parent, which in turn causes all its other children to be removed, since you've got another CascadeType.ALL on Shop.campaigns).

Also, if you never want to update the Shop while saving the Campaign, what is the purpose of the following code?

if (shop.getShopId() > 0 && shop.getShopId() != null) {
    shop.setCode(campaignDto.getShopCode());
    shop.setCurrency(campaignDto.getShopCurrency());
    shop.setCurrencyIsoCode(campaignDto.getShopCurrencyIso());
    shop.setValidFrom(campaignDto.getValidFrom());
    shop.setValidTill(campaignDto.getValidTill());
    shop.setCreatedAt(OffsetDateTime.now());
    shop.setSalesOrganisationId(campaignDto.getSalesOrganisationId());
}

If you truly do not want to update the Shop, just remove that part. As a side note, you might want to swap the conditions in the if clause.

Finally, if you're not seeing updates to the CAMPAIGN table, it might be because there are no actual changes to the Campaign entity. In general, Hibernate will retrieve the current state and compare it with the updated Entity to avoid unnecessary UPDATE statements.

First of all, drop the `CascadeType.ALL` from `Campaign.shop` and you'll stop seeing updates being made to the `SHOP` table. Secondly, could you verify that `mapper.mapReverse(campaignDto)` does indeed return an instance of `Campaign` whose values differ from the ones already in the database? — crizzis, Apr 30 '18 at 12:48
Also, as I said, Hibernate must first retrieve the current state of `Campaign` as part of the merging process, so seeing a `SELECT` stament upon calling `campaignRepository.saveAndFlush(campaign)` is to be expected — crizzis, Apr 30 '18 at 12:49

JPA/hibernate is generating the wrong SQL statement

1 Answers1