Hadoop drop table to return false if table does not exist

[itachi-sharingan]

closes #3051

[RussellSpitzer]

I would prefer keeping the returns in the same if block as the other return statement. I tend to always lean towards single or very closely located exits from functions so it's clear that you either return this or that.

Like if you had it as

if (lastMetadata == null) {
  LOG.error(....)
  return false
} else {
  return fs.delete(...)
}

[itachi-sharingan]

Have tried doing something similar in the latest commit, please review.

[rdblue]

Looks like this is fixed to me.

[RussellSpitzer]

Looks good to me, a few minor comments but we definitely need some tests to make sure we can hit all of the different return points in this code. I would suggest adding some to TestHadoopCatalog.java

[itachi-sharingan]

Have added test for the other case of exit because of non iceberg table, please review.

[RussellSpitzer]

I would suggest adding one more test to make sure we return false when the table doesn't exist at all (no folder or anything)

[rdblue]

@itachi-sharingan, could you add the test that Russell suggested and fix the failing CI checks? Otherwise, I think this is about ready to go.

[itachi-sharingan]

@rdblue have fixed the failing CI.
@RussellSpitzer have added the check for non existent directory drop returning false in the same test.
Hence, I am closing this pr.
Moreover, can someone please suggest what would be a good issue to pick up next?

[rdblue]

Thanks, @itachi-sharingan! I reopened this so that I could merge it. There's no need to close a PR when it is finished.

If you're looking for another contribution, #2488 is a good one! Thanks for contributing!