Recently I noticed a long(ish) SQL query happening during checkout (checkout/onepage/index)
The same calculation could be seen in the cart, when adding products.
On the site, the SQL could take up to 2ms to process, thus adding unwanted delay to the end user experience. The SQL also ran multiple times.
My investigations reveal a potential bug in how magento deals with quotes and tax calculation, where no address has yet been selected.
So firstly what I see (using New Relic):
In the cart: (this is when additing the initial first product into the cart, and I am not logged in)
as can be seen above, the calculation added 2.82ms to the cart display
and entering checkout (/checkout/onestep/index), still not logged in:
so, in checkout we get 1.290 ms added to the process.
Digging some details from New Relic I can get the actual query, as well as a trace that resulted into the query:
Granted, the query could be having issues due to improperly configured database server, or resource issues etc. Unfortunately the server aspects are outside of my control. To note is that this is happening for every page load to the areas in question, so it is not an intermittend issue.
Thus, looking at the above trace data, I can see that magento is calculating the tax to apply onto the order, using (shipping) address details.
BUT I HAVE NOT SPECIFIED A SHIPPING (OR BILLING) ADDRESS (YET), AND I AM NOT LOGGED IN.
So why the need to calculate the tax?
So lets trace the actual code, and see whats going on: I decided to use the cart area, as the code will be easier to trace.
- Upon initial page load, the cart IndexAction is loaded (Mage_Checkout_CartController::IndexAction)
This then initialize the cart, and issue a $cart->save() - The save leads to '$this->getQuote()->collectTotals();'
- collectTotals will then lead to the tax totals collection (rightfully so) in Mage_Tax_Model_Sales_Total_Quote_Shipping::collect
Here we have this code:
$priceIncludesTax = $this->_config->shippingPriceIncludesTax($store);
if ($priceIncludesTax) {
$this->_areTaxRequestsSimilar = $calc->compareRequests($addressTaxRequest, $storeTaxRequest);
}
In the above $addressTaxRequest is the shipping address details, and $storeTaxRequest is the stores default address (as set in tax configuration) -
This then leads to Mage_Tax_Model_Calculation::compareRequests. It is in this routine that the SQL inquestion gets fired. The logic that leads to teh sql is this:
$country = $first->getCountryId() == $second->getCountryId();
// "0" support for admin dropdown with --please select--
$region = (int)$first->getRegionId() == (int)$second->getRegionId();
$postcode= $first->getPostcode() == $second->getPostcode();
$taxClass= $first->getCustomerClassId() == $second->getCustomerClassId();if ($country && $region && $postcode && $taxClass) {
return true;
}
/**
* Compare available tax rates for both requests
*/
$firstReqRates = $this->_getResource()->getRateIds($first); <=== QUERY HERE
$secondReqRates = $this->_getResource()->getRateIds($second); <=== AND AGAIN !
if ($firstReqRates === $secondReqRates) {
return true;
}
So, lets analyze what is happening there:
- Compare the countries. In this store this results TRUE as they are both set to "AU"
- Compare the region ids (casted to int) - Results in TRUE ( The shipping address is set to '0', and the store addres is set to 'Western Australia', casted to INT = 0)
- Compare the postcodes - Results in FALSE (The shipping address is set to '0', and the store address is set to '6000')
- Compare the taxClass - Results in TRUE (they are both set to '2')
- Test if all is true, then RETURN TRUE - which does not validate, as POSTCODE is FALSE
- Get the tax rate for the shipping address (this is the SQL in question)
- Get the tax class for the store address (this is a second run of the SQL in question)
- If they match - RETURN TRUE - This validates, and we result in TRUE back to the calling routine
Thus, the question is:
Why is tax calculated, when no address was selected?
When the Shipping Address hits the comparison in the routine above, the postcode is set to * - Why?
Tracing the code backwards I get the following:
in Mage_Tax_Model_Sales_Total_Quote_Shipping::collect() the Address To calculate Tax against is set as such:
$addressTaxRequest = $calc->getRateRequest(
$address,
$address->getQuote()->getBillingAddress(),
$address->getQuote()->getCustomerTaxClassId(),
$store
);
That code leads to this: Mage_Tax_Model_Calculation::getRateRequest(), and in this routine we end up on this:
case 'default':
$address
->setCountryId(Mage::getStoreConfig(
Mage_Tax_Model_Config::CONFIG_XML_PATH_DEFAULT_COUNTRY,
$store))
->setRegionId(Mage::getStoreConfig(Mage_Tax_Model_Config::CONFIG_XML_PATH_DEFAULT_REGION, $store))
->setPostcode(Mage::getStoreConfig(
Mage_Tax_Model_Config::CONFIG_XML_PATH_DEFAULT_POSTCODE,
$store));
break;
Thus, from the above, I can see that the Postcode for the Tax Calculation Address should get a default set via the configuration path 'Mage_Tax_Model_Config::CONFIG_XML_PATH_DEFAULT_POSTCODE' ( 'tax/defaults/postcode' )
Checking my configuration, I see the default value for postocde is set to * in the tax configuration
By simply adjusting the values in admin for 'Default Tax Destination Calculation' (to match the values set in the Shipping Settings->Origin address setting), I managed to eliminate this initial tax calculation, and shave of (on avarage) 2ms from Cart and initial checkout index page display!
What is strange is that the magento configuration guide says these options are only applicable if the Default Country is set to United States. Is this a bug in the docs?